mehdi_amini created this revision.
mehdi_amini added reviewers: chandlerc, rsmith.
mehdi_amini added a subscriber: cfe-commits.
This intended as a debugging/development flag only.
https://reviews.llvm.org/D28385
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/Co
mehdi_amini created this revision.
mehdi_amini added reviewers: chandlerc, rsmith.
mehdi_amini added subscribers: cfe-commits, dexonsmith.
Herald added a reviewer: tstellarAMD.
Herald added a subscriber: nhaehnle.
Amongst other, this will help LTO to correctly handle/honor files
compiled with O0,
mehdi_amini updated this revision to Diff 83391.
mehdi_amini added a comment.
Herald added a subscriber: wdng.
Remove spurious change
https://reviews.llvm.org/D28404
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/CodeGenOptions.def
clang/lib/CodeGen/CodeGenMo
mehdi_amini created this revision.
mehdi_amini added reviewers: echristo, chandlerc.
mehdi_amini added a subscriber: cfe-commits.
Clang was initializing the TargetMachine with CodeGenOpt::Default
for https://reviews.llvm.org/owners/package/1/. This change is aligning it on
llc:
-O0: OptLevel = C
mehdi_amini updated this revision to Diff 83404.
mehdi_amini added a comment.
clang-format
https://reviews.llvm.org/D28409
Files:
clang/lib/CodeGen/BackendUtil.cpp
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/Code
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D28362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291276: Use CodegenOpts::less when creating a TargetMachine
for clang `-O1` (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D28409?vs=83404&id=83408#toc
Repository:
rL LL
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#638217, @probinson wrote:
> Maybe instead, pass a flag to enable setting optnone on everything when the
> driver sees `-O0 -flto`?
I'm not fond of this: limiting discrepancy between LTO and non-LTO reduces the
LTO specific bugs a
mehdi_amini added a comment.
> In order to simplify distributed build system integration, where actions
may be scheduled before the Thin Link which determines the list of
objects selected by the linker. The gold plugin currently will emit
0-sized index files for objects not selected by the link,
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#638299, @probinson wrote:
> In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D28404#638217, @probinson wrote:
> >
> > > The patch as-is obviously has a massive testing cost, and it's ea
mehdi_amini updated this revision to Diff 83433.
mehdi_amini added a comment.
Herald added a subscriber: jholewinski.
Fix minsize issue (conditional was reversed)
https://reviews.llvm.org/D28404
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/CodeGenOptions.def
mehdi_amini updated this revision to Diff 83441.
mehdi_amini added a comment.
Herald added subscribers: dschuff, jfb.
Fix one more conflicts with always_inline, and change some test check lines
https://reviews.llvm.org/D28404
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/cla
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291300: Add a cc1 option to force disabling lifetime-markers
emission from clang (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D28385?vs=83321&id=83443#toc
Repository:
mehdi_amini updated this revision to Diff 83459.
mehdi_amini added a comment.
Address comments: reorganize the way ShouldAddOptNone is handled, hopefully
make it more easy to track.
Also after talking with Chandler on IRC, the source attribute "cold" does
not add the LLVM IR attribute "optsize"
mehdi_amini marked 6 inline comments as done.
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:760-762
Fn->removeFnAttr(llvm::Attribute::NoInline);
+ Fn->removeFnAttr(llvm::Attribute::OptimizeNone);
Fn->addFnAttr(llvm::Attribute::AlwaysI
mehdi_amini marked 2 inline comments as done.
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:900
+ ShouldAddOptNone &= !D->hasAttr();
+ if (ShouldAddOptNone) {
+B.addAttribute(llvm::Attribute::OptimizeNone);
probinson wro
mehdi_amini updated this revision to Diff 83468.
mehdi_amini added a comment.
Address Paul's comment (remove useless block and add period to end comment)
https://reviews.llvm.org/D28404
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/CodeGenOptions.def
clang/l
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912
// OptimizeNone wins over OptimizeForSize and MinSize.
F->removeFnAttr(llvm::Attribute::OptimizeForSize);
F->removeFnAttr(llvm::Attribute::MinSize);
probinso
mehdi_amini updated this revision to Diff 83480.
mehdi_amini added a comment.
Forgot to update test/CodeGen/attr-naked.c
https://reviews.llvm.org/D28404
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/CodeGenOptions.def
clang/lib/CodeGen/CGOpenMPRuntime.cpp
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912
// OptimizeNone wins over OptimizeForSize and MinSize.
F->removeFnAttr(llvm::Attribute::OptimizeForSize);
F->removeFnAttr(llvm::Attribute::MinSize);
chandler
mehdi_amini updated this revision to Diff 83510.
mehdi_amini added a comment.
Rebase
https://reviews.llvm.org/D24688
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/nobuiltin.c
clang/test/CodeGenCUDA/flush-denormals.cu
clang/test/CodeGen
mehdi_amini added a comment.
In https://reviews.llvm.org/D24688#638835, @pcc wrote:
> Didn't we figure out in the end that this could be a function attribute
> instead?
We did? You wrote in PR30403: "I had a brief look at what it would take to have
a per-function TLI, and I'm not convinced th
mehdi_amini added a comment.
In https://reviews.llvm.org/D24688#639158, @jyknight wrote:
> Since this requires everyone generating llvm IR to add this module attribute
> for optimal codegen,
Not sure yet how it'll end up, I'll revamp this.
See also: http://lists.llvm.org/pipermail/cfe-dev/2017
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: CMakeLists.txt:113
+check_linker_flag("-Wl,--section-ordering-file,${CLANG_ORDER_FILE}"
+ LINKER_ORDER_FILE_WORKS)
+ endif()
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#639874, @probinson wrote:
> Over the weekend I had a thought: Why is -O0 so special here? That is,
> after going to all this trouble to propagate -O0 to LTO, how does this
> generalize to propagating -O1 or any other specific -O
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#640046, @probinson wrote:
> In https://reviews.llvm.org/D28404#639887, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D28404#639874, @probinson wrote:
> >
> > > Over the weekend I had a thought: Why is -O0 so special here? T
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#640170, @probinson wrote:
> In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D28404#640046, @probinson wrote:
> >
> > > "I don't care" doesn't seem like much of a principle.
> >
> >
> >
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote:
> Also, that's not practicable: what if I have an LTO static library for which
> I don't have the source, now if I build my own file with -O0 -flto I can't
> link anymore.
Also: LTO is required for som
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#640284, @probinson wrote:
> In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D28404#640170, @probinson wrote:
> >
> > > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote:
>
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#640297, @probinson wrote:
> Sorry, you lost me. CFI is part of DWARF and we do DWARF perfectly well
> without LTO (and at O0).
This CFI: http://clang.llvm.org/docs/ControlFlowIntegrity.html
https://reviews.llvm.org/D28404
__
mehdi_amini added a comment.
Actually, as mentioned before, I could be fine with making `O0` incompatible
with LTO, however security features like CFI (or other sort of whole-program
analyses/instrumentations) requires LTO.
https://reviews.llvm.org/D28404
___
mehdi_amini added a comment.
> I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds
> welcome) which would set the default for the pragma to 'off'. How is that
> different than what you wanted for `-O0`? It is defined in terms of an
> existing pragma, which is WAY easier t
mehdi_amini added a comment.
Thanks, I'll investigate function attributes then!
https://reviews.llvm.org/D24688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#641538, @probinson wrote:
> > - optnone isn't *really* no optimizations: clearly this is true, but then
> > neither is -O0. We run the always inliner, a couple of other passes, and we
> > run several parts of the code generators op
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#641597, @probinson wrote:
> In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote:
>
> > As I stand right now, there hasn't been any correction.
> > I still consider the fact that `optnone` wouldn't produce the "same"
> >
mehdi_amini added a comment.
In https://reviews.llvm.org/D28404#641632, @probinson wrote:
> In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote:
>
> > If we want to support `-O0 -flto` and `optnone` it the way to convey this
> > to the optimizer, I don't see the alternative.
>
>
> opts
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#700574, @hfinkel wrote:
> In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D30920#700133, @pcc wrote:
> > >
> > >
mehdi_amini updated this revision to Diff 91804.
mehdi_amini added a comment.
Update: we set deployment by default even when not testing the system library.
It is intentional because a REQUIRE in a test might be because an issue with
libc for example.
So for now the default is the OS on which the
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#702979, @pirama wrote:
> The driver (accepts, but) ignores Os and other optimization flags for non-lto
> link-only actions. That it has an effect for LTO is seems to be an
> implementation detail. Since optimization flags are com
mehdi_amini added a comment.
It would be reasonable to *not* forward any flag to the linker (plugin), but
not to rewrite them with a different meaning.
https://reviews.llvm.org/D30920
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
mehdi_amini added a comment.
(By the way, this is what is done on Darwin: the -O flag is *ignored* for the
link step invocation, because we couldn't find a "right way" of correctly
honoring it that would be logical to the user in all case).
https://reviews.llvm.org/D30920
__
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#703082, @hfinkel wrote:
> In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote:
>
> > Yes, the issue is only about how the driver accepts Os for the link even
> > though it has no effect
> > (O0/https://reviews.llvm.org/ow
mehdi_amini added a comment.
The fundamental difference, is that Os/Oz especially are treated as
`optimizations directive` that are independent of the pass pipeline:
instructing that "loop unroll should not increase size" is independent of
*where* is loop unroll inserted in the pipeline.
The i
mehdi_amini created this revision.
Herald added a subscriber: Prazek.
https://reviews.llvm.org/D31114
Files:
clang/lib/CodeGen/BackendUtil.cpp
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/BackendUtil.cpp:982
std::unique_ptr OS) {
+ EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
+
tejohnson wrote:
> Why did this move?
"History
mehdi_amini added a comment.
In https://reviews.llvm.org/D31114#704649, @tejohnson wrote:
> I think you won't get the correct handling of -emit-llvm and -emit-llvm-bc
> since we don't get the handling for Backend_Emit* in
> EmitAssemblyHelper::EmitAssembly.
I was not trying to achieve this. A
mehdi_amini added a comment.
In https://reviews.llvm.org/D31114#704728, @tejohnson wrote:
> In https://reviews.llvm.org/D31114#704726, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D31114#704649, @tejohnson wrote:
> >
> > > I think you won't get the correct handling of -emit-llvm and
> >
mehdi_amini added a comment.
In https://reviews.llvm.org/D31114#704748, @tejohnson wrote:
> In https://reviews.llvm.org/D31114#704733, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D31114#704728, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D31114#704726, @mehdi_amini wrote:
>
mehdi_amini created this revision.
Herald added a subscriber: mgorny.
The linker would fail because the list of reexported symbols contains
new/delete operators.
https://reviews.llvm.org/D31272
Files:
lib/CMakeLists.txt
Index: lib/CMakeLists.txt
mehdi_amini added a subscriber: joerg.
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#710209, @hfinkel wrote:
> Yes, we should do this. I don't understand why this is tricky. Actually, I
> think having these kinds of decisions explicit in the code of the
> transformations would
mehdi_amini added a comment.
In https://reviews.llvm.org/D31272#711804, @EricWF wrote:
> I'm a bit confused by the description of this change. Libc++ has been
> enabling the new/delete definitions in its dylib since forever and I've never
> experienced a link error. Did you mean to say the link
mehdi_amini added a comment.
Not that I'm claiming to understand why we're changing strategy when we have
the command line tools installed or not in this case ;)
(CC @beanz if he know by any chance!)
https://reviews.llvm.org/D31272
___
cfe-commits
mehdi_amini added a comment.
Strange. So installing the command line tools is not enough. It has to be that
CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe?
Anyway, if you're exporting through the ABI list and have
LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS I expect the link to fail
mehdi_amini added a comment.
In https://reviews.llvm.org/D31272#711877, @EricWF wrote:
> In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote:
>
> > Strange. So installing the command line tools is not enough. It has to be
> > that CMAKE_OSX_SYSROOT is only defined on Apple internal ins
mehdi_amini added a comment.
OK, I figured, it is because I have this revision locally on top of this one:
https://reviews.llvm.org/D30765 ; and I can't submit the latter without the
change here.
https://reviews.llvm.org/D31272
___
cfe-commits mai
mehdi_amini added inline comments.
Comment at:
test/std/language.support/support.types/byteops/xor.assign.pass.cpp:13
+
+// XFAIL: c++98, c++03, c++11, c++14
+
EricWF wrote:
> Nit. These should be `// UNSUPPORTED`. `XFAIL` is for bugs we need to fix.
I have mixe
mehdi_amini created this revision.
As we're trying to setup testing / bots for all shipping version of libc++
on macOS/iOS, we'll need to be able to pass a path to where to find the
dylib for each previous version of the OS.
https://reviews.llvm.org/D31486
Files:
utils/libcxx/test/config.py
mehdi_amini added inline comments.
Comment at: lib/CMakeLists.txt:155
+# We can't use the "-reexported_symbols_list" when we build the
+# new/delete operators as part of the dylib: the linker would fail.
+set(OSX_RE_EXPORT_LINE
"-Wl,-reexport_library,${CM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299053: libc++ testing: allow to provide a path for
`use_system_cxx_lib` (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D31486?vs=93432&id=93444#toc
Repository:
rL LLVM
mehdi_amini closed this revision.
mehdi_amini added a comment.
r299052 ; let me know if you want to improve the comment in any way.
https://reviews.llvm.org/D31272
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299054: Reexport operator new / delete from libc++abi
(authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D30765?vs=91091&id=93445#toc
Repository:
rL LLVM
https://reviews.ll
mehdi_amini closed this revision.
mehdi_amini added a comment.
Was committed a while back in r297798
https://reviews.llvm.org/D17469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D31508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
mehdi_amini added a comment.
This bot is broken:
http://green.lab.llvm.org/green/job/libcxx_master_cmake_32/61/
https://reviews.llvm.org/D16541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
mehdi_amini added a comment.
I believe considering the goal of moving to using per-instruction FMF and kills
the global backend option, this leads to a bitcode upgrade issue: OpenCL (or
other) bitcode that were generated bitcode don't have the right FMF and will be
upgraded conservatively.
Perf
mehdi_amini added a comment.
Ping?
https://reviews.llvm.org/D31739
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini added inline comments.
Comment at: cfe/trunk/CMakeLists.txt:531
+ if(BOOTSTRAP_LLVM_ENABLE_LLD)
+add_dependencies(clang-bootstrap-deps lld)
+ elseif(LLVM_BINUTILS_INCDIR)
I come back to this a bit late, sorry, but I'm not sure I unde
mehdi_amini added inline comments.
Comment at: docs/ReleaseNotes.rst:69
+ called from cold callsite.
+
We need to mention ThinLTO in the Release Notes, so thanks for initiating this.
We likely should go with a paragraph summarizing the improvements, I don't k
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM, thanks.
Comment at: docs/ReleaseNotes.rst:70
+
+ - Integration with profile data. When available, profile data enables mo
mehdi_amini added a comment.
Oh I didn't notice that we were in the clang Release Notes, actually I've only
updated the LLVM ones in the past.
So yes, I think this should be in LLVM as well, I don't know if it should
differ in some way?
https://reviews.llvm.org/D28746
_
mehdi_amini added a comment.
Yes, thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D28821
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini added a comment.
In https://reviews.llvm.org/D28843#649207, @tejohnson wrote:
> > TODO: avoid breaking Darwin.
>
> How does this break Darwin?
IIUC, we don't use the new LTO API, which is handling this gracefully.
https://reviews.llvm.org/D28843
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2
+// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden
-emit-llvm-bc -o %t %s
+// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+
-
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2
+// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden
-emit-llvm-bc -o %t %s
+// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+
-
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2
+// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden
-emit-llvm-bc -o %t %s
+// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+
-
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2
+// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden
-emit-llvm-bc -o %t %s
+// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+
-
mehdi_amini added a comment.
In https://reviews.llvm.org/D28867#649760, @vsk wrote:
> Ah ok, so it sounds like a better approach would be to split the missing
> record message into a separate off-by-default warning.
I'm in favor of this.
https://reviews.llvm.org/D28867
___
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D28877
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D28349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.
mehdi_amini created this revision.
These accessors maps directly to the command line option.
https://reviews.llvm.org/D29065
Files:
clang/include/clang/Driver/Driver.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/Tools.cpp
Index: clang/lib/Driver/Tools.cpp
==
mehdi_amini created this revision.
https://reviews.llvm.org/D29066
Files:
clang/lib/Driver/Tools.cpp
Index: clang/lib/Driver/Tools.cpp
===
--- clang/lib/Driver/Tools.cpp
+++ clang/lib/Driver/Tools.cpp
@@ -8435,9 +8435,13 @@
//
mehdi_amini added inline comments.
Comment at: clang/include/clang/CodeGen/BackendUtil.h:51
+ llvm::Expected
+ FindThinLTOModule(llvm::MemoryBufferRef MBRef);
}
Indentation seems strange?
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:841
mehdi_amini updated this revision to Diff 85530.
mehdi_amini added a comment.
Add a test
https://reviews.llvm.org/D29066
Files:
clang/lib/Driver/Tools.cpp
clang/test/Driver/embed-bitcode.c
Index: clang/test/Driver/embed-bitcode.c
===
mehdi_amini added a comment.
In https://reviews.llvm.org/D29065#654336, @steven_wu wrote:
> I think you should add a new API for embedBitcodeMarkerEnabled() or revert
> the name the APIs to before https://reviews.llvm.org/rL287084.
I'm missing where https://reviews.llvm.org/rL287084 changed th
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292960: Split isUsingLTO() outside of embedBitcodeInObject()
and embedBitcodeMarkerOnly… (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D29065?vs=85511&id=85605#toc
Reposi
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292961: Forward -bitcode_process_mode to ld64 in marker-only
mode (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D29066?vs=85530&id=85606#toc
Repository:
rL LLVM
https:
mehdi_amini added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:452
+ if (CodeGenOpts.DebugInfoForProfiling)
+getModule().addModuleFlag(llvm::Module::Warning, "DebugInfoForProfiling",
1);
Why should we warn on mismatch?
https://reviews.llvm
mehdi_amini added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:452
+ if (CodeGenOpts.DebugInfoForProfiling)
+getModule().addModuleFlag(llvm::Module::Warning, "DebugInfoForProfiling",
1);
danielcdh wrote:
> mehdi_amini wrote:
> > Why should w
mehdi_amini added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:452
+ if (CodeGenOpts.DebugInfoForProfiling)
+getModule().addModuleFlag(llvm::Module::Warning, "DebugInfoForProfiling",
1);
mehdi_amini wrote:
> danielcdh wrote:
> > mehdi_amini
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: lib/AST/ASTContext.cpp:8909
+
+// Itanium ABI (& MSVC seems to do similarly) requires static locals in
+// inline functions to be emitted
mehdi_amini added inline comments.
Comment at: cfe/trunk/lib/Driver/Tools.cpp:6464
if (C.getDriver().isSaveTempsEnabled() &&
- !C.getDriver().embedBitcodeInObject() && isa(JA))
+ !C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() &&
+ isa(JA))
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM.
Can you apply the patch in LLVM (libDemangle) as well please?
Repository:
rL LLVM
https://reviews.llvm.org/D27985
___
cfe-com
mehdi_amini added a comment.
In https://reviews.llvm.org/D27985#660354, @davidb wrote:
> In https://reviews.llvm.org/D27985#660029, @mehdi_amini wrote:
>
> > LGTM.
> >
> > Can you apply the patch in LLVM (libDemangle) as well please?
>
>
> Thank you Mehdi. I have an LLVM (libDemangle) with an ide
mehdi_amini added a comment.
> My concern with that approach is that we'd just end up playing whack-a-bug.
> It's possible that this fix puts us in a similar situation, but I'm honestly
> at a loss for a better approach. ¯\_(ツ)_/¯ (...And I'd kinda prefer to play
> whack-a-bug with "clang cou
mehdi_amini added a comment.
Ping? This is hitting 4.0
https://reviews.llvm.org/D29469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini created this revision.
This can cause a compile failure in cases like:
double log(double);
namespace foo {
namespace log {}
}
using namespace foo;
void bar(int i) {
log((double)i);
}
https://reviews.llvm.org/D29804
Files:
libcxx/include/math.h
Index: libcxx/include/math.
mehdi_amini added a comment.
In https://reviews.llvm.org/D24644#792168, @fhahn wrote:
> I'd like to fix PR22999 and was wondering if you think adding a
> function-section attribute to the IR would be a viable solution?
>
> When doing LTO, we could add the same function-section to each function i
mehdi_amini created this revision.
This enables better optimization, I don't if it is legal c++11 though.
https://reviews.llvm.org/D34992
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
Index: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
===
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:12
+// CHECK: @_ZN3Foo4BAR2E = external constant i32,
+ static const int BAR2 = 43;
+};
Note: I'm not sure if the standard allows us to assume that we can fold the
mehdi_amini accepted this revision.
mehdi_amini added a comment.
Make sure your commit message correctly reference the LLVM change.
Comment at: test/CodeGen/ms-barriers-intrinsics.c:39
#endif
-
Don't remove: file should end with a new line I believe.
https:/
201 - 300 of 367 matches
Mail list logo
