mehdi_amini (Mehdi AMINI)
User

Projects

User does not belong to any projects.
User Since
Apr 30 2013, 5:34 PM (203 w, 2 d)

Recent Activity

Today

mehdi_amini added a comment to D26959: llvm-strings - dumping strings from LLVM bitcode.

As previously mentioned, if this absolutely requires that the file not be treated opaquely, then we should be putting this functionality into another tool. Perhaps llvm-bc would be a good home for this. I'd really rather LLVM-strings be kept simple and treat all input as opaque.

Thu, Mar 23, 2:26 PM

Yesterday

mehdi_amini created D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete.
Wed, Mar 22, 5:21 PM
mehdi_amini added a comment to D20116: Add speculatable function attribute.

That troubles (and worries) me as well.

Why?

Wed, Mar 22, 4:08 PM
mehdi_amini added a comment to D20116: Add speculatable function attribute.

If we go with (2), then we've admitted that "dead code" (that is, code that is not run) can influence program behavior, which I find troubling.

Wed, Mar 22, 2:56 PM

Tue, Mar 21

mehdi_amini added a comment to D31232: [IR] Use a binary search in DataLayout::getAlignmentInfo.

It is not clear to me why an associative container instead of a vector wouldn't be even easier to use?

Tue, Mar 21, 8:46 PM
mehdi_amini accepted D31230: IPO: Const correctness for summaries passed into passes..

LGTM

Tue, Mar 21, 8:06 PM
mehdi_amini accepted D31226: IR: Fix a race condition in type id clients of ModuleSummaryIndex..

LGTM.

Tue, Mar 21, 7:59 PM
mehdi_amini added inline comments to D31226: IR: Fix a race condition in type id clients of ModuleSummaryIndex..
Tue, Mar 21, 5:43 PM
mehdi_amini added a comment to D20116: Add speculatable function attribute.

readnone etc. are different from speculatable, in that once you mark a call site as speculatable you've the said call site as speculatable throughout the lifetime of the program (since, by definition, it can be arbitrarily speculated). readnone, readonly etc. do not have that property.

Tue, Mar 21, 4:38 PM
mehdi_amini added inline comments to D30468: Simplify the CFG after loop pass cleanup..
Tue, Mar 21, 3:27 PM
mehdi_amini added inline comments to D31217: Disable loop unrolling and icp in SamplePGO ThinLTO compile phase.
Tue, Mar 21, 3:20 PM
mehdi_amini added inline comments to D31217: Disable loop unrolling and icp in SamplePGO ThinLTO compile phase.
Tue, Mar 21, 3:01 PM

Mon, Mar 20

mehdi_amini added inline comments to D31164: [IR] Add AllowContract to FastMathFlags.
Mon, Mar 20, 9:10 PM
mehdi_amini added inline comments to D31164: [IR] Add AllowContract to FastMathFlags.
Mon, Mar 20, 9:09 PM
mehdi_amini added inline comments to D31164: [IR] Add AllowContract to FastMathFlags.
Mon, Mar 20, 8:30 PM
mehdi_amini accepted D31105: [Support] Add a function to MD5 a file's contents..

LGTM. See one comment inline.

Mon, Mar 20, 4:11 PM
mehdi_amini added a comment to D31105: [Support] Add a function to MD5 a file's contents..

How does it compare to the SHA1 API? Are these in sync? (If not any good reason they're not?)

Mon, Mar 20, 4:06 PM
mehdi_amini committed rL298291: Fix year (2016->2017) for 4.0.0 in "Release Emails" section.
Fix year (2016->2017) for 4.0.0 in "Release Emails" section
Mon, Mar 20, 12:43 PM
mehdi_amini added a comment to D31027: [ThinLTO] Add support for emitting minimized bitcode for thin link.

I did some testing with this change. The cumulative size of the bitcode files that need to be sent to the remote node performing the thin link drops from 20G to 2.5G.

Mon, Mar 20, 10:33 AM

Sat, Mar 18

mehdi_amini added a comment to D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends.

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. And your approach of an "OptimizeOnly" or "DisableCodeGen" on the lot::Config for this purpose makes sense.

I'm confused - are you saying you now think D31100 and D31101 are the right approach?

I believe your patch is the right approach when clang needs to get the optimized IR, which is the case for -emit-llvm/-emit-bc. I believe that it shouldn't do that when it expects an object file.

What about for -S, which presumably maps to Backend_EmitAssembly?

Sat, Mar 18, 1:26 PM
mehdi_amini added a comment to D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends.

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. And your approach of an "OptimizeOnly" or "DisableCodeGen" on the lot::Config for this purpose makes sense.

I'm confused - are you saying you now think D31100 and D31101 are the right approach?

Sat, Mar 18, 11:46 AM
mehdi_amini added a comment to D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends.

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.

Sat, Mar 18, 10:29 AM
mehdi_amini added inline comments to D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends.
Sat, Mar 18, 10:26 AM

Fri, Mar 17

mehdi_amini added a comment to D31100: [LTO] Allow client to skip code gen.

So I rather get clang to initialize the TargetOptions in the LTO config. I think it should be easy to extract EmitAssemblyHelper::CreateTargetOptions out of EmitAssemblyHelper::CreateTargetMachine and reuse it for this purpose.

That is doable, but I don't see the advantage of duplicating the logic in EmitAssemblyHelper (e.g. CreateTargetMachine), just so we can invoke the codegen through LTO, since we have to be able to do this outside LTO in EmitAssemblyHelper in the non-LTO case in clang. We have to set up TargetOptions in either case, see below.

Not clear which duplicating logic you're referring to right now? I'm talking about *reusing* the logic (OK I may miss something because I haven't looked deep enough).

Fri, Mar 17, 11:13 PM
mehdi_amini created D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends.
Fri, Mar 17, 11:12 PM
mehdi_amini added a comment to D28759: [ExecutionDepsFix] Improve clearance calculation for loops.

We are seeing this in a real stage2 build of clang and we have to fix the buildbot!

Fri, Mar 17, 10:58 PM
mehdi_amini added a comment to D31100: [LTO] Allow client to skip code gen.

So I rather get clang to initialize the TargetOptions in the LTO config. I think it should be easy to extract EmitAssemblyHelper::CreateTargetOptions out of EmitAssemblyHelper::CreateTargetMachine and reuse it for this purpose.

That is doable, but I don't see the advantage of duplicating the logic in EmitAssemblyHelper (e.g. CreateTargetMachine), just so we can invoke the codegen through LTO, since we have to be able to do this outside LTO in EmitAssemblyHelper in the non-LTO case in clang. We have to set up TargetOptions in either case, see below.

Fri, Mar 17, 9:27 PM
mehdi_amini added a comment to D31063: LTO: Fix a potential race condition in the caching API..

I'm quite amazed how you guys narrowed this down so quickly! Well done...

Fri, Mar 17, 3:11 PM
mehdi_amini added a comment to D31100: [LTO] Allow client to skip code gen.

We noticed that when invoking the thinBackend via clang (for the distributed build case) that flags like -ffunction-sections and -emit-llvm were not having the intended effect. This could have been fixed by setting up the TargetOptions and the CodeGenFileType in the LTO Config, but since clang already has handling for all of this, it is straightforward to just let it do the handling.

Well, I'm not convinced it is the right thing to do. It is the most straightforward thing to do for the distributed case, but it creates a discrepancy with the non-distributed case, and it won't allow to honor -ffunction-sections this way.

Can you clarify the concern? We do pass down -ffunction-sections correctly in the non-distributed case, e.g. via the gold plugin. The gold plugin which is only handling LTO doesn't have any built in code gen handling (anymore with the new LTO API).

OK, then how do we pass down -ffunction-sections to the LTO API in the non-distributed case? I expect the same flow to happen here.

See AddGoldPlugin in clang for where we translate it into the equivalent llvm internal option passed to the plugin. It eventually gets used to init the target options in the Config in the plugin.

Fri, Mar 17, 2:44 PM
mehdi_amini added a comment to D31100: [LTO] Allow client to skip code gen.

We noticed that when invoking the thinBackend via clang (for the distributed build case) that flags like -ffunction-sections and -emit-llvm were not having the intended effect. This could have been fixed by setting up the TargetOptions and the CodeGenFileType in the LTO Config, but since clang already has handling for all of this, it is straightforward to just let it do the handling.

Well, I'm not convinced it is the right thing to do. It is the most straightforward thing to do for the distributed case, but it creates a discrepancy with the non-distributed case, and it won't allow to honor -ffunction-sections this way.

Can you clarify the concern? We do pass down -ffunction-sections correctly in the non-distributed case, e.g. via the gold plugin. The gold plugin which is only handling LTO doesn't have any built in code gen handling (anymore with the new LTO API).

Fri, Mar 17, 2:14 PM
mehdi_amini added a comment to D31100: [LTO] Allow client to skip code gen.

We noticed that when invoking the thinBackend via clang (for the distributed build case) that flags like -ffunction-sections and -emit-llvm were not having the intended effect. This could have been fixed by setting up the TargetOptions and the CodeGenFileType in the LTO Config, but since clang already has handling for all of this, it is straightforward to just let it do the handling.

Fri, Mar 17, 2:06 PM
mehdi_amini added a comment to D31096: LTO, Support: Add a safeguard against pruning a directory that is not a cache directory..
In D31096#704183, @pcc wrote:

Ivan suggested that we can just use a hidden directory inside the user-supplied directory as the cache directory. I like that approach better, so I'm abandoning this.

Fri, Mar 17, 1:40 PM
mehdi_amini added a comment to D31096: LTO, Support: Add a safeguard against pruning a directory that is not a cache directory..

The idea is to protect against data loss if the user supplies an incorrect cache directory.

Fri, Mar 17, 1:38 PM
mehdi_amini added a comment to D31070: Turn some C-style vararg into variadic templates.

Oh we only need clang/lld/lldb to work, I'll check this before committing.

Fri, Mar 17, 11:24 AM
mehdi_amini accepted D31070: Turn some C-style vararg into variadic templates.

LGTM, thanks for the cleanup!

Fri, Mar 17, 10:41 AM

Thu, Mar 16

mehdi_amini added inline comments to D31063: LTO: Fix a potential race condition in the caching API..
Thu, Mar 16, 5:39 PM
mehdi_amini accepted D31063: LTO: Fix a potential race condition in the caching API..

LGTM. Thanks!

Thu, Mar 16, 5:37 PM
mehdi_amini added inline comments to D30879: Distinguish between code pointer size and DataLayout::getPointerSize() in DWARF info generation .
Thu, Mar 16, 3:55 PM
mehdi_amini added a comment to D30879: Distinguish between code pointer size and DataLayout::getPointerSize() in DWARF info generation .

Note that the existing code was unsigned AsmPrinter::getPointerSize() const { return TM.getPointerSize(); }, so it is not introducing necessarily new uses of the TM here.

Thu, Mar 16, 3:52 PM
mehdi_amini added a comment to D31057: Make Argument::getArgNo() constant time, not O(#args).

It actually shows up as the 23rd hottest leaf function in a 13s sample of LTO of llc.

Thu, Mar 16, 3:39 PM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

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.

Thu, Mar 16, 3:34 PM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

Yes, the issue is only about how the driver accepts Os for the link even though it has no effect (O0/O1/O2/O3 *will* have an effect though).

Do we agree that the desired endpoint here is that all optimization flags are encoded in the IR somehow? If so, in this desired end state, will it will be true that -O[n] will have some affect on an LTO link step?

Thu, Mar 16, 12:55 PM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

(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).

Thu, Mar 16, 11:39 AM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

It would be reasonable to *not* forward any flag to the linker (plugin), but not to rewrite them with a different meaning.

Thu, Mar 16, 11:22 AM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

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 compiler-only options, and Clang already silently (without a warning) ignores these flags during link-only invocations, silently transforming them when passing to the plugin seems reasonable.

Thu, Mar 16, 11:22 AM
mehdi_amini accepted D31045: LTO: Create temporary cache files in the cache directory instead of $TMPDIR..

That's nice! LGTM.

Thu, Mar 16, 11:19 AM

Wed, Mar 15

mehdi_amini added inline comments to D31020: Support: Add a cache pruning policy parser..
Wed, Mar 15, 9:04 PM
mehdi_amini added inline comments to D31021: ELF: Add cache pruning support..
Wed, Mar 15, 8:14 PM
mehdi_amini added inline comments to D31021: ELF: Add cache pruning support..
Wed, Mar 15, 8:08 PM
mehdi_amini accepted D31020: Support: Add a cache pruning policy parser..

Neat! LGTM.

Wed, Mar 15, 8:03 PM
mehdi_amini accepted D31009: Support: Simplify the CachePruning API. NFCI..

LGTM.

Wed, Mar 15, 3:59 PM
mehdi_amini accepted D30584: CodeGen: Use the source filename as the argument to .file, rather than the module ID..

(Sorry for the delay I didn't see this one)

Wed, Mar 15, 8:35 AM

Tue, Mar 14

mehdi_amini committed rL297798: Add deployment knobs to tests (for Apple platforms).
Add deployment knobs to tests (for Apple platforms)
Tue, Mar 14, 6:12 PM
mehdi_amini updated the diff for D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).

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 test runs.

Tue, Mar 14, 6:07 PM
mehdi_amini accepted D30333: Split SimplifyCFG to run obscuring switch transforms only during last phase.

LGTM.

Tue, Mar 14, 5:07 PM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.
In D30920#700133, @pcc wrote:

Until everything is converted to using size attributes, it seems like a correct fix for the bug is to accept these options in the gold-plugin and pass through the LTO API to the PassManagerBuilder.

Not necessarily. There is no requirement (from a correctness perspective) that -Os at link time should exactly match the behaviour of -Os at compile time.

Sure, but there is certainly a perception that optimization flags affecting the non-LTO pipeline should similarly affect the LTO pipeline. LTO should be transparent to the user, so if -Os behaves one way without LTO, it seems problematic to me if it behaves a different way with LTO.

That being said, agree that the best way to enforce that is to pass the relevant flags through the IR. (On the flip side, if the user passes -O1 to the link step, it does get passed through to the plugin and affects the LTO optimization pipeline...)

I agree that I don't like the discrepancy: the driver should *not* drop -Os silently if it passes down -O1/-O2/-O3, a warning is the minimum.

I don't like the discrepancy either, and I agree that we should be passing these other flags through the IR as well (even though, in the face of inlining, there is some ambiguity as to what the flags would mean). That having been said, I don't see the value in the warning. Forcing users to endure a warning solely because they use LTO and use -Os or -Oz for all of their compilation steps, is not friendly.

Tue, Mar 14, 10:25 AM
mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.
In D30920#700133, @pcc wrote:

Until everything is converted to using size attributes, it seems like a correct fix for the bug is to accept these options in the gold-plugin and pass through the LTO API to the PassManagerBuilder.

Not necessarily. There is no requirement (from a correctness perspective) that -Os at link time should exactly match the behaviour of -Os at compile time.

Sure, but there is certainly a perception that optimization flags affecting the non-LTO pipeline should similarly affect the LTO pipeline. LTO should be transparent to the user, so if -Os behaves one way without LTO, it seems problematic to me if it behaves a different way with LTO.

That being said, agree that the best way to enforce that is to pass the relevant flags through the IR. (On the flip side, if the user passes -O1 to the link step, it does get passed through to the plugin and affects the LTO optimization pipeline...)

Tue, Mar 14, 8:06 AM

Mon, Mar 13

mehdi_amini added a comment to D30920: Do not pass -Os and -Oz to the Gold plugin.

Agree with @pcc. Unless anyone has a need to have "perfect" support for Os, this is the right direction.

Mon, Mar 13, 6:52 PM
mehdi_amini added a comment to D21723: [RFC] Enhance synchscope representation.

I think I mentioned in the past using target specific strings instead of integer in textual and bitcode serialization, why was this ruled out? This was intended to addressed such issues (and make the textual IR more readable)

I do not recall original reasons we did not go with target specific strings, but just thinking from the top of my head:

  • Existing synchronization scope field is represented as 32 bits unsigned field in the bitcode
Mon, Mar 13, 5:06 PM
mehdi_amini added a reviewer for D30920: Do not pass -Os and -Oz to the Gold plugin: tejohnson.
Mon, Mar 13, 4:14 PM
mehdi_amini added a comment to D30791: Add support for -fno-builtin to LTO and ThinLTO on Darwin.
In D30791#699844, @pcc wrote:

If this is just a temporary solution for one specific application, does it really need first class support in the clang driver? Would it not be sufficient for you to pass -Wl,-mllvm,-lto-nobuiltin at link time?

Mon, Mar 13, 4:03 PM
mehdi_amini added a comment to D30791: Add support for -fno-builtin to LTO and ThinLTO on Darwin.

It looks like this is part of the required fix for PR30403? I guess the other part is to add the module flag to be set during the FE, and use that to set this flag?

Mon, Mar 13, 2:43 PM
mehdi_amini added inline comments to D21723: [RFC] Enhance synchscope representation.
Mon, Mar 13, 2:31 PM
mehdi_amini added inline comments to D21723: [RFC] Enhance synchscope representation.
Mon, Mar 13, 1:59 PM
mehdi_amini accepted D30880: Stop using isPhysRegModified() in RegUsageInfoCollector.
Mon, Mar 13, 1:56 PM
mehdi_amini added inline comments to D21723: [RFC] Enhance synchscope representation.
Mon, Mar 13, 12:55 PM
mehdi_amini added inline comments to D30880: Stop using isPhysRegModified() in RegUsageInfoCollector.
Mon, Mar 13, 12:48 PM
mehdi_amini added inline comments to D30880: Stop using isPhysRegModified() in RegUsageInfoCollector.
Mon, Mar 13, 12:36 PM
mehdi_amini added a comment to D21723: [RFC] Enhance synchscope representation.

We need an entry in the bitcode compatibility test as well.

Mon, Mar 13, 11:29 AM
mehdi_amini added inline comments to D30880: Stop using isPhysRegModified() in RegUsageInfoCollector.
Mon, Mar 13, 10:38 AM

Sat, Mar 11

mehdi_amini added a comment to D30842: Add documentation on debug counters to Programmers Manual..

LGTM, but see a few inline comments.

Sat, Mar 11, 9:06 PM

Fri, Mar 10

mehdi_amini added a comment to D30792: Use callback for internalizing linked symbols..

Off topic, but since this is a rev lock change with LLVM, you can to all of in a single revision with: http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo

Fri, Mar 10, 8:21 AM
mehdi_amini added inline comments to D30333: Split SimplifyCFG to run obscuring switch transforms only during last phase.
Fri, Mar 10, 7:41 AM

Thu, Mar 9

mehdi_amini added inline comments to D30333: Split SimplifyCFG to run obscuring switch transforms only during last phase.
Thu, Mar 9, 10:44 PM
mehdi_amini added inline comments to D30333: Split SimplifyCFG to run obscuring switch transforms only during last phase.
Thu, Mar 9, 10:17 PM
mehdi_amini added a comment to D30738: Don't internalize llvm GV's with InternalizeLinkedSymbols.
In D30738#697051, @pcc wrote:

The fix can be simplified to

if (P.first().startswith("llvm."))
  continue;
Thu, Mar 9, 4:07 PM
mehdi_amini added inline comments to D30738: Don't internalize llvm GV's with InternalizeLinkedSymbols.
Thu, Mar 9, 2:45 PM
mehdi_amini created D30791: Add support for -fno-builtin to LTO and ThinLTO on Darwin.
Thu, Mar 9, 1:47 PM

Wed, Mar 8

mehdi_amini committed rL297359: Do not use parallel link jobs with ThinLTO on Green Dragon.
Do not use parallel link jobs with ThinLTO on Green Dragon
Wed, Mar 8, 9:15 PM
mehdi_amini committed rL297352: Jenkins build.py: add a --thinlto option..
Jenkins build.py: add a --thinlto option.
Wed, Mar 8, 5:37 PM
mehdi_amini committed rL297351: Add a cmake cache file for a stage-2 build with ThinLTO.
Add a cmake cache file for a stage-2 build with ThinLTO
Wed, Mar 8, 5:30 PM
mehdi_amini added a comment to D30765: Reexport operator new / delete from libc++abi.

Note: this is already shipping like this for ~2 years in our OSes.

Wed, Mar 8, 3:51 PM
mehdi_amini created D30765: Reexport operator new / delete from libc++abi.
Wed, Mar 8, 3:43 PM
mehdi_amini added a comment to D30738: Don't internalize llvm GV's with InternalizeLinkedSymbols.

I can see how this feature can be useful, but as Teresa mentioned it seems easier from a client point of view to:

Wed, Mar 8, 8:59 AM

Tue, Mar 7

mehdi_amini added inline comments to D28759: [ExecutionDepsFix] Improve clearance calculation for loops.
Tue, Mar 7, 10:49 PM
mehdi_amini added inline comments to D28759: [ExecutionDepsFix] Improve clearance calculation for loops.
Tue, Mar 7, 10:47 PM
mehdi_amini updated subscribers of D28759: [ExecutionDepsFix] Improve clearance calculation for loops.
Tue, Mar 7, 10:45 PM
mehdi_amini added a comment to D26348: Allow convergent attribute for function arguments.

I meant that T1 would run with { v1 = 0; cond = true; } and T1 with with { v1 = 1; cond = false; }

Tue, Mar 7, 11:47 AM
mehdi_amini added a comment to D26348: Allow convergent attribute for function arguments.

//Example 1: Safe to fold.
void foo(int v, bool cond) {

if (cond) {
  // token1 = statepoint()
  // bar(v, token1); // v needs to be convergent
} else {
  // token2 = statepoint()
  // bar(v, token2);  // v needs to be convergent
}
token_merged = statepoint()
bar(v, token_merged); // v needs to be convergent

}

Tue, Mar 7, 11:45 AM
mehdi_amini accepted D30585: Fix test and add missing return for llvm-lto2 error case.

But are these two changes related?

Tue, Mar 7, 10:23 AM
mehdi_amini added a comment to D30588: [LTO] Add module asm uses to the llvm.compiler.used before merging.

(IIUC).

Tue, Mar 7, 9:57 AM
mehdi_amini added a comment to D30588: [LTO] Add module asm uses to the llvm.compiler.used before merging.

*internal* functions referenced from ASM need to be in llvm.compiler.used, regular global functions don't.

Tue, Mar 7, 9:57 AM
mehdi_amini added inline comments to D30676: Add llvm::sys::fs::remove_directories().
Tue, Mar 7, 9:30 AM
mehdi_amini added a comment to D26348: Allow convergent attribute for function arguments.

My point is that one can't choose performance and sacrifices knowingly correctness, and then complains about a LLVM bug that need to be fixed. The bug is in the shader compiler in the first place!

I missed the rationale for this statement, but this does not seem obviously correct to me. Even marking the intrinsics as having unknown side effects does not seem strong enough to ensure the semantics required here - we don't really have a strong definition for "unknown side effects", AFAIK, but I'd not have thought that even unknown side effects on Foo would prevent the folding of:

if (cond) {
  Tmp1 = Foo(v [convergent])
} else {
  Tmp2 = Foo(v [convergent])
}
Tmp3 = phi [Tmp1, Tmp2]
Tue, Mar 7, 8:47 AM

Mon, Mar 6

mehdi_amini accepted D30676: Add llvm::sys::fs::remove_directories().

LGTM

Mon, Mar 6, 10:05 PM
mehdi_amini added inline comments to D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).
Mon, Mar 6, 7:50 PM
mehdi_amini added inline comments to D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).
Mon, Mar 6, 7:02 PM
mehdi_amini updated the diff for D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).

Add back the python changes (files were moved, I lost them in the rebase)

Mon, Mar 6, 7:02 PM
mehdi_amini updated the diff for D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).

Rebase

Mon, Mar 6, 6:25 PM
mehdi_amini commandeered D17469: [libcxx] Add deployment knobs to tests (for Apple platforms).
Mon, Mar 6, 6:25 PM