efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (97 w, 4 d)

Recent Activity

Fri, Jun 22

efriedma added inline comments to D47655: [MachineOutliner] Don't outline sequences where x16/x17/nzcv are live across.
Fri, Jun 22, 4:10 PM
efriedma committed rL335400: [LoopReroll] Rewrite induction variable rewriting..
[LoopReroll] Rewrite induction variable rewriting.
Fri, Jun 22, 4:03 PM
efriedma closed D45191: [LoopReroll] Rewrite induction variable rewriting..
Fri, Jun 22, 4:03 PM
efriedma added a comment to D48504: [WIP] Add InsertionOrderSet, with constant-time insertion and removal..

I could have used DenseMapInfo::getTombstoneKey() here instead of Optional, I think. But making remove() swap might still be worthwhile; it would give random-access iteration, and more flexibility with the keys (with a different underlying hashtable, we could allow keys which don't have tombstones).

Fri, Jun 22, 1:11 PM
efriedma added a comment to D48372: [MemorySSAUpdater] Remove deleted trivial Phis from active workset.

We do linear lookups for NonOptPhis.erase() anyway

Fri, Jun 22, 12:51 PM
efriedma created D48504: [WIP] Add InsertionOrderSet, with constant-time insertion and removal..
Fri, Jun 22, 12:42 PM
efriedma added inline comments to D48332: [AArch64] Add custom lowering for v4i8 trunc store.
Fri, Jun 22, 11:12 AM
efriedma added inline comments to D48128: [ARM] Parallel DSP IR Pass.
Fri, Jun 22, 11:09 AM

Thu, Jun 21

efriedma accepted D48453: [SCEV] Re-apply r335197 (with Polly fixes)..

LGTM on the polly bits.

Thu, Jun 21, 2:15 PM
efriedma added inline comments to D47655: [MachineOutliner] Don't outline sequences where x16/x17/nzcv are live across.
Thu, Jun 21, 2:10 PM
efriedma added a comment to D45916: Enable MachineOutliner by default under -Oz for AArch64.

This is getting complicated enough that I'd like to see it split into two patches; one for the MachineOutliner option/configuration changes, and one to actually change the default for AArch64.

Thu, Jun 21, 2:05 PM
efriedma accepted D48330: [GVN] Avoid casting a vector of size less than 8 bits to i8.

LGTM

Thu, Jun 21, 2:01 PM
efriedma added a comment to D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.

Also, LTO currently calls TargetLibraryInfo::disableAllFunctions to avoid this problem.

Thu, Jun 21, 2:00 PM
efriedma added a comment to D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.

Maybe instead of trying to turn off optimizations on internal builtin functions, we should instead try to make optimizations not treat functions with local linkage as builtin? Not sure. This came up before on llvmdev, but we didn't really decide one way or the other.

Thu, Jun 21, 1:56 PM
efriedma added inline comments to D48330: [GVN] Avoid casting a vector of size less than 8 bits to i8.
Thu, Jun 21, 1:30 PM
efriedma accepted D48172: [CostModel][AArch64] Add some initial costs for SK_Select and SK_PermuteSingleSrc.

LGTM

Thu, Jun 21, 12:51 PM
efriedma added a comment to D47895: llvm: Add support for "-fno-delete-null-pointer-checks".

Please fix lib/Analysis/BasicAliasAnalysis.cpp to allow null pointers to alias other objects. Please fix lib/Analysis/LoopAccessAnalysis.cpp to allow a loop to dereference null. Please fix isGEPKnownNonNull in lib/Analysis/ValueTracking.cpp to allow objects located at null. Please fix ConstantFoldScalarCall in lib/Analysis/ConstantFolding.cpp to allow objects located at null. Please fix Argument::hasNonNullAttr in lib/IR/Function.cpp to allow objects at null.

Thu, Jun 21, 12:16 PM
efriedma accepted D47874: [SCEVExp] Advance found insertion point until we find a non-dbg instruction..

LGTM assuming you're planning to regression-test this somehow.

Thu, Jun 21, 12:02 PM
efriedma added inline comments to D48421: [IPSCCP] Try to replace phis before they are removed by changeToUnreachable..
Thu, Jun 21, 12:01 PM
efriedma added inline comments to D48372: [MemorySSAUpdater] Remove deleted trivial Phis from active workset.
Thu, Jun 21, 11:46 AM
efriedma added inline comments to D48332: [AArch64] Add custom lowering for v4i8 trunc store.
Thu, Jun 21, 11:27 AM
efriedma added inline comments to D48128: [ARM] Parallel DSP IR Pass.
Thu, Jun 21, 11:09 AM
efriedma added a comment to D48330: [GVN] Avoid casting a vector of size less than 8 bits to i8.

For the test, I mean something like the following:

Thu, Jun 21, 11:00 AM

Wed, Jun 20

efriedma added a comment to D47927: [RISCV] Custom lower ISD::{U,S}{ADD,SUB}O nodes.

It would probably make sense to change SelectionDAGLegalize::ExpandNode to use this lowering for SADDO/SSUBO, instead of making this target-specific. The target-independent version uses approximately the same operations anyway, just in a less efficient way.

Wed, Jun 20, 6:46 PM
efriedma updated subscribers of D48283: [SCEV] Properly solve quadratic equations.

See also https://bugs.llvm.org//show_bug.cgi?id=37211 .

Wed, Jun 20, 5:05 PM
efriedma added inline comments to D48172: [CostModel][AArch64] Add some initial costs for SK_Select and SK_PermuteSingleSrc.
Wed, Jun 20, 2:08 PM
efriedma updated subscribers of D48376: DAG: Fold out selects with an undef operand.
Wed, Jun 20, 12:58 PM
efriedma added a comment to D48332: [AArch64] Add custom lowering for v4i8 trunc store.

I tried your suggestion, but without further tuning in vector lowering this does not yield much gain on a vector store operation.

Wed, Jun 20, 12:06 PM
efriedma added inline comments to D48372: [MemorySSAUpdater] Remove deleted trivial Phis from active workset.
Wed, Jun 20, 11:21 AM

Tue, Jun 19

efriedma accepted D46560: [SelectionDAG] Don't crash on inline assembly errors when the inline assembly return type is a struct..

Nothing should ask for the result of an inline asm which produces zero values; that's fine. (The chain is handled separately.)

Tue, Jun 19, 6:42 PM
efriedma accepted D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..

Maybe the use list order isn't affected in this exact testcase. In general, the order ssa.copy intrinsics are inserted matters, though.

Tue, Jun 19, 1:49 PM
efriedma accepted D48305: [IR] Introduce helpers to skip debug instructions (NFC).

LGTM

Tue, Jun 19, 1:16 PM
efriedma added a comment to D48332: [AArch64] Add custom lowering for v4i8 trunc store.

I wonder if we should prefer to widen <2 x i8> and <4 x i8> to <8 x i8> instead of promoting to <4 x i16>. It would make stores like this a bit cheaper. Maybe an interesting experiment at some point (mostly just modifying AArch64TargetLowering::getPreferredVectorAction, I think, and seeing what happens to the generated code).

Tue, Jun 19, 1:10 PM
efriedma accepted D48029: [DAGCombine] Fix alignment for offset loads/stores.

LGTM

Tue, Jun 19, 12:31 PM
efriedma accepted D48170: ARM: use "add" instead of "orr" for code size.

LGTM

Tue, Jun 19, 12:16 PM
efriedma added inline comments to D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..
Tue, Jun 19, 12:08 PM
efriedma added a comment to D48330: [GVN] Avoid casting a vector of size less than 8 bits to i8.

It should be possible to implement this sort of coercion with an appropriate cast, rather than just bailing out, I think. But I won't block the patch on that.

Tue, Jun 19, 12:01 PM
efriedma added inline comments to D48305: [IR] Introduce helpers to skip debug instructions (NFC).
Tue, Jun 19, 11:14 AM

Mon, Jun 18

efriedma added a comment to D48305: [IR] Introduce helpers to skip debug instructions (NFC).

I don't think donothing counts as a meta-instruction, by your definition; yes, it has no effects and no operands, but it isn't ignored by the optimizer (it generally just gets erased, but that's not the same as ignoring it).

Mon, Jun 18, 5:42 PM
efriedma committed rL335004: [ARM] Thumb2 constant cmp testcases..
[ARM] Thumb2 constant cmp testcases.
Mon, Jun 18, 5:18 PM
efriedma committed rL335003: [ARM] Testcase for Thumb1 cmp with constants..
[ARM] Testcase for Thumb1 cmp with constants.
Mon, Jun 18, 5:16 PM
efriedma committed rL335002: [ARM] Add Thumb1 coverage for cmn testcases..
[ARM] Add Thumb1 coverage for cmn testcases.
Mon, Jun 18, 5:14 PM
efriedma committed rL335001: [ARM] Testcase for missed optimization for masking..
[ARM] Testcase for missed optimization for masking.
Mon, Jun 18, 5:14 PM
efriedma committed rL335000: [ARM] Testcase for missed optimization with i16 compare..
[ARM] Testcase for missed optimization with i16 compare.
Mon, Jun 18, 5:14 PM
efriedma added a comment to D48029: [DAGCombine] Fix alignment for offset loads/stores.

The getExtLoad/getTruncStore API seems really confusing, but I guess there's no simple way to fix it.

Mon, Jun 18, 4:22 PM
efriedma updated subscribers of D47963: [LangRef] Clarify that nnan and ninf don't produce undef or poison..

Only if the fcmp is marked with nnan or ninf right?

Mon, Jun 18, 1:58 PM
efriedma requested changes to D48270: [ARM] Check for unaligned access via bitcasts.

In IR, if a pointer operand doesn't have the alignment given by the "align" annotation, the behavior is undefined. (If you don't explicitly specify the alignment, it defaults to the alignment specified in the DataLayout.) Similarly, in a MachineFunction, if a memory operation accesses an address which doesn't have the alignment given in the MMO, the behavior is undefined.

Mon, Jun 18, 1:00 PM
efriedma accepted D46198: [LoopInterchange] Move PHI handling to adjustLoopBranches..

LGTM

Mon, Jun 18, 12:46 PM
efriedma added a comment to D48239: [LangRef] Clarify meaning of "dereferencable" attribute/metadata..

It's straightforward to define the alternative semantics where it only applies at the point of the call/load. And it would still be useful to the optimizer. But the optimizer code would have to be written from scratch; the existing getPointerDereferenceableBytes API isn't usable with an attribute like that. It's probably worth doing at some point, though: we could prove other interesting things with the context-sensitive analysis, though. For example, we could prove that a pointer is dereferenceable using a previous load or store operation.)

Mon, Jun 18, 12:42 PM
efriedma added a comment to D48170: ARM: use "add" instead of "orr" for code size.

Also, I'd like to see some test coverage for the immediate patterns.

Mon, Jun 18, 12:23 PM
efriedma added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Mon, Jun 18, 12:20 PM
efriedma added a comment to D45321: [atomics] Fix runtime calls for misaligned atomics.

Did a bit more research into the GNU libatomic behavior. As far as I can tell, my initial assessment was correct: libatomic never uses unaligned atomic operations. However, it uses a trick which makes this a little complicated to test: it promotes small atomic operations to pointer size before performing the alignment check. So, for example, libatomic supports lock-free operations with size=3, depending on the input pointer. (On x86, it still only promotes up to pointer size, even though larger lock-free atomics are available; not sure why.)

Mon, Jun 18, 11:52 AM

Fri, Jun 15

efriedma added a comment to D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..

Oh, wait, you mean in the "Reviewers" field; nevermind.

Fri, Jun 15, 6:03 PM
efriedma added a comment to D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..

@klimek I think he just replied to the llvm-commits email.

Fri, Jun 15, 6:00 PM
efriedma added a comment to D47196: [Time-report ](2): Recursive timers in Clang.

Can you give an example of what the output looks like?

Fri, Jun 15, 3:02 PM
efriedma added a comment to D47747: [LangRef] Clarify "undefined" for various instructions..

Fast math changes: https://reviews.llvm.org/D47963
Dereferenceable: https://reviews.llvm.org/D48239

Fri, Jun 15, 2:43 PM
efriedma created D48239: [LangRef] Clarify meaning of "dereferencable" attribute/metadata..
Fri, Jun 15, 2:41 PM
efriedma updated the diff for D47854: [LangRef] Clarify semantics of load metadata..

Actually, move the dereferenceable changes out of this patch; it should all go together, separately.

Fri, Jun 15, 2:01 PM
efriedma added a comment to D47854: [LangRef] Clarify semantics of load metadata..

Any other comments on this? The !dereferenceable changes might be a bit controversial.

Fri, Jun 15, 1:37 PM
efriedma added a comment to D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..

because values have slightly different labels

Fri, Jun 15, 12:51 PM
efriedma added inline comments to D46198: [LoopInterchange] Move PHI handling to adjustLoopBranches..
Fri, Jun 15, 12:32 PM
efriedma added a comment to D47895: llvm: Add support for "-fno-delete-null-pointer-checks".

I went through those files but many of these addressSpace() == 0 (and != 0) checks are not for null pointers but for disabling/skipping some memory optimizations. And it is not clear to me if moving all of those checks to the new NullPointerIsDefined function makes sense there.

Fri, Jun 15, 11:54 AM
efriedma added a reviewer for D48229: [NFC][SCEV] Add tests related to bit masking (PR37793): sanjoy.
Fri, Jun 15, 11:35 AM
efriedma added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Fri, Jun 15, 11:17 AM

Thu, Jun 14

efriedma committed rL334779: [compiler-rt] [builtins] Don't build __atomic_* by default..
[compiler-rt] [builtins] Don't build __atomic_* by default.
Thu, Jun 14, 4:27 PM
efriedma committed rCRT334779: [compiler-rt] [builtins] Don't build __atomic_* by default..
[compiler-rt] [builtins] Don't build __atomic_* by default.
Thu, Jun 14, 4:27 PM
efriedma closed D47606: [compiler-rt] [builtins] Don't build __atomic_load etc. by default..
Thu, Jun 14, 4:27 PM
efriedma committed rL334777: Make uitofp and sitofp defined on overflow..
Make uitofp and sitofp defined on overflow.
Thu, Jun 14, 4:03 PM
efriedma closed D47807: Make uitofp and sitofp defined on overflow..
Thu, Jun 14, 4:03 PM
efriedma added inline comments to D48170: ARM: use "add" instead of "orr" for code size.
Thu, Jun 14, 2:43 PM
efriedma added a comment to D47963: [LangRef] Clarify that nnan and ninf don't produce undef or poison..

It has to be one of undefined behavior, poison, undef, or an unspecified value as described here; there is no other way for something to be "undefined" in LLVM IR, unless you're proposing to introduce a new form of undefined-ness specifically for nnan/ninf.

Thu, Jun 14, 12:44 PM
efriedma added a comment to D45330: [IPSCCP] Use PredicateInfo to propagate facts from cmp instructions..

My first guess would be some sort of non-determinism.

Thu, Jun 14, 11:48 AM
efriedma added a comment to D48074: [ARM] Enable useAA() for the in-order Cortex-R52.

I'm generally not a fan of having features like this, which have widespread implications, turned on only for certain target CPUs; it tends to make it much harder to find bugs, since the code gets little testing. But I guess this is okay for now.

Thu, Jun 14, 11:23 AM

Wed, Jun 13

efriedma added a comment to D48057: easing the constraint for isNegatibleForFree and GetNegatedExpression.

HonorSignDependentRounding is pretty clearly a failed experiment; I'd suggest killing it completely (in a separate patch).

Wed, Jun 13, 3:24 PM
efriedma added a comment to D48128: [ARM] Parallel DSP IR Pass.

On targets with NEON, we should prefer NEON vectorization (vmlal etc.) over DSP instructions in almost all cases. Is this intended specifically for targets which don't have NEON?

Wed, Jun 13, 2:40 PM
efriedma added a comment to D48040: Implement constexpr __builtin_*_overflow.

Oh, sorry, I mixed up the two values. I meant that you should add a couple testcases to ensure the stored value is correct on overflow.

Wed, Jun 13, 12:14 PM
efriedma added inline comments to D47895: llvm: Add support for "-fno-delete-null-pointer-checks".
Wed, Jun 13, 12:12 PM
efriedma accepted D48040: Implement constexpr __builtin_*_overflow.

I'd like to see a couple of testcases ensuring the return value is correct on overflow (we had a problem with that for __builtin_mul_overflow in the past).

Wed, Jun 13, 11:42 AM
efriedma added a comment to D48131: [RISCV] Implement codegen for cmpxchg on RV32I.

You can introduce a target-specific SelectionDAG node without adding a corresponding IR intrinsic. See ISD::FIRST_TARGET_MEMORY_OPCODE.

Wed, Jun 13, 11:28 AM
efriedma added a comment to D48122: [SimplifyCFG] Hoist common if-then-else code if used by two-entry PHI nodes.

GVNHoist is generally pretty stable; my team has used it to build a lot of code without any issues. There are a few known bugs (https://bugs.llvm.org/show_bug.cgi?id=37791, https://bugs.llvm.org/show_bug.cgi?id=36635, https://bugs.llvm.org/show_bug.cgi?id=37445 ) that we probably need to fix before turning it on by default.

Wed, Jun 13, 11:18 AM
efriedma added a comment to D47895: llvm: Add support for "-fno-delete-null-pointer-checks".

Tests look good, at first glance.

Wed, Jun 13, 11:06 AM

Tue, Jun 12

efriedma added a comment to D47917: [ARM] Lower llvm.ctlz.i32 to a libcall when clz is not available..

__clzsi2 is provided by libgcc/compiler-rt, so it should be generally available. I guess it might be possible to construct an environment where it isn't, but I'm not sure how.

Tue, Jun 12, 2:54 PM
efriedma accepted D47831: [DAGCombiner] Recognize more patterns for ABS.

LGTM

Tue, Jun 12, 2:35 PM
efriedma accepted D48053: Correct behavior of __builtin_*_overflow and constexpr..

LGTM

Tue, Jun 12, 2:05 PM
efriedma added inline comments to D47655: [MachineOutliner] Don't outline sequences where x16/x17/nzcv are live across.
Tue, Jun 12, 1:59 PM

Mon, Jun 11

efriedma accepted D47725: [SelectionDAG] Provide default expansion for rotates.

LGTM

Mon, Jun 11, 5:03 PM
efriedma added inline comments to D47831: [DAGCombiner] Recognize more patterns for ABS.
Mon, Jun 11, 5:01 PM
efriedma added a comment to D47655: [MachineOutliner] Don't outline sequences where x16/x17/nzcv are live across.

We should probably use this functionality to allow outlining instructions which access/modify SP, in cases where we don't need to spill LR. But that's probably better to do as a followup; it could have some unexpected effects on the heuristics.

Mon, Jun 11, 4:42 PM
efriedma added a comment to D47917: [ARM] Lower llvm.ctlz.i32 to a libcall when clz is not available..

I'm not checking for optsize because I don't think it makes sense to inline even when optimizing for speed... although maybe that's not right. The current code for Thumb1 is something like the following (which is essentially computing popcount(nextpoweroftwo(x)-1)).

Mon, Jun 11, 3:59 PM
efriedma accepted D46552: [AArch64] Support reserving x20 register.

LGTM

Mon, Jun 11, 3:08 PM
efriedma updated the diff for D47963: [LangRef] Clarify that nnan and ninf don't produce undef or poison..

Clarify that the value produced isn't undef. (It's more like freeze(undef).)

Mon, Jun 11, 2:44 PM
efriedma added a reviewer for D48053: Correct behavior of __builtin_*_overflow and constexpr.: rsmith.
Mon, Jun 11, 2:37 PM
efriedma added inline comments to D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR.
Mon, Jun 11, 2:26 PM
efriedma added inline comments to D48040: Implement constexpr __builtin_*_overflow.
Mon, Jun 11, 1:54 PM
efriedma added inline comments to D47854: [LangRef] Clarify semantics of load metadata..
Mon, Jun 11, 1:15 PM
efriedma added inline comments to D48040: Implement constexpr __builtin_*_overflow.
Mon, Jun 11, 12:31 PM

Fri, Jun 8

efriedma updated subscribers of D47681: [DAGCombiner] Bug 31275- Extract a shift from a constant mul or udiv if a rotate can be formed.
Fri, Jun 8, 3:42 PM
efriedma created D47963: [LangRef] Clarify that nnan and ninf don't produce undef or poison..
Fri, Jun 8, 3:33 PM
efriedma committed rL334326: [LangRef] fptosi and fptoui return poison on overflow..
[LangRef] fptosi and fptoui return poison on overflow.
Fri, Jun 8, 2:37 PM
efriedma closed D47851: [LangRef] fptosi and fptoui return poison on overflow..
Fri, Jun 8, 2:37 PM