efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (61 w, 5 d)

Recent Activity

Today

efriedma added inline comments to D38128: Handle COPYs of physregs better (regalloc hints).
Mon, Oct 16, 2:09 PM
efriedma added inline comments to D38942: [DAG] Promote ADDCARRY / SUBCARRY.
Mon, Oct 16, 12:15 PM
efriedma added inline comments to D38952: [ARM] Allow unrolling on multi-block loops..
Mon, Oct 16, 12:06 PM

Thu, Oct 12

efriedma committed rL315619: [DWARF] Fix bad comparator in sortGlobalExprs..
[DWARF] Fix bad comparator in sortGlobalExprs.
Thu, Oct 12, 1:54 PM
efriedma closed D38830: [DWARF] Fix bad comparator in sortGlobalExprs. by committing rL315619: [DWARF] Fix bad comparator in sortGlobalExprs..
Thu, Oct 12, 1:54 PM
efriedma added a comment to D38299: [ARM] Honor -mfloat-abi for libcall calling convention.

Ping.

Thu, Oct 12, 12:47 PM
efriedma added inline comments to D38128: Handle COPYs of physregs better (regalloc hints).
Thu, Oct 12, 12:28 PM
efriedma added a comment to D34515: [ARM] Materialise some boolean values to avoid a branch.

This patch would be easier to review if you would commit the changes to add new tests and RUN lines separately.

Thu, Oct 12, 12:16 PM
efriedma added inline comments to D38662: [BasicAA] Support arbitrary pointer sizes (and fix an overflow bug).
Thu, Oct 12, 11:57 AM
efriedma updated subscribers of D38848: {ARM} IfConversion does not handle un-analyzable branch correctly.
Thu, Oct 12, 11:50 AM
efriedma updated subscribers of D38848: {ARM} IfConversion does not handle un-analyzable branch correctly.
Thu, Oct 12, 11:47 AM

Wed, Oct 11

efriedma accepted D38637: [InstSimplify] don't let poison inhibit an easy fold.

My only potential concern here is that we could end up blocking optimizations because we're folding to undef rather than zero... but that's probably rare enough that it doesn't matter. LGTM.

Wed, Oct 11, 7:11 PM
efriedma added inline comments to D38531: Improve clamp recognition in ValueTracking..
Wed, Oct 11, 6:58 PM
efriedma added a comment to D38830: [DWARF] Fix bad comparator in sortGlobalExprs..

The obvious ways to reduce the testcase aren't working for me, and I don't really have a day to sink into it now. If you really need it, it'll probably take me a couple weeks to find the time. (The construct that's triggering it is something like https://bugs.llvm.org//show_bug.cgi?id=34921, but there's some funny interaction GlobalMerge and the exact order of the input which means simple transformations make the problem disappear.)

Wed, Oct 11, 6:06 PM
efriedma added a comment to D38830: [DWARF] Fix bad comparator in sortGlobalExprs..

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

Wed, Oct 11, 5:27 PM
efriedma created D38830: [DWARF] Fix bad comparator in sortGlobalExprs..
Wed, Oct 11, 4:45 PM
efriedma accepted D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.

LGTM.

Wed, Oct 11, 12:49 PM
efriedma added inline comments to D38662: [BasicAA] Support arbitrary pointer sizes (and fix an overflow bug).
Wed, Oct 11, 12:15 PM

Tue, Oct 10

efriedma updated subscribers of D38765: Don't replace constants with constants in GVN.
Tue, Oct 10, 5:52 PM
efriedma accepted D37898: [TargetLowering] Correctly track NumFixedArgs field of CallLoweringInfo.

LGTM

Tue, Oct 10, 4:30 PM
efriedma added inline comments to D38662: [BasicAA] Support arbitrary pointer sizes (and fix an overflow bug).
Tue, Oct 10, 4:14 PM
efriedma added a comment to D38717: Wunused-variable does not detect unused condition variable in if statement.

Please make sure the title (subject line) for a patch reflects the actual contents of the change, as opposed to referring to Bugzilla; a lot of people skim cfe-commits, so you want to make sure interested reviewers find you patch.

Tue, Oct 10, 3:48 PM
efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Tue, Oct 10, 3:05 PM
efriedma added a comment to D38662: [BasicAA] Support arbitrary pointer sizes (and fix an overflow bug).

because I believe that it is not uncommon to use i64 indexing expressions in 32-bit code

Tue, Oct 10, 2:56 PM
efriedma added a comment to D38756: [x86] use an insert op to put one variable element into a constant of vectors.

I tried to work-around the FP issue by creating a constant pool load, but that got a bit ugly - I had to call LowerConstantPool() to create a legal vector of constants.

Tue, Oct 10, 2:45 PM
efriedma added a comment to D38756: [x86] use an insert op to put one variable element into a constant of vectors.

This doesn't happen with int constants...because int constants are legal but FP constants are not?

Tue, Oct 10, 2:23 PM
efriedma added a comment to D38154: [PassManager] Run global opts after the inliner.

The downside of the original version of the patch is most likely worse codesize and runtime performance. If the inliner isn't interleaved correctly, the heuristics become less accurate; the inliner depends on the simplification passes to get rid of dead and redundant code.

Tue, Oct 10, 12:59 PM
efriedma updated subscribers of D38558: [JumpThreading] Preserve DT and LVI across the pass..
Tue, Oct 10, 9:25 AM

Fri, Oct 6

efriedma added a comment to D38637: [InstSimplify] don't let poison inhibit an easy fold.

The exact definition of poison is still getting refined, but it's different from undef. undef is a bit-wise property, which is why ComputeKnownBits has to be careful around it. poison works differently; essentially, any arithmetic or logical operation which has poison as an input produces poison, no matter what the other input is. So it doesn't matter what ComputeKnownBits returns for a known poison value.

Fri, Oct 6, 3:59 PM
efriedma added a comment to D36706: DAGCombiner: Add form of isFPExtFree to check uses.

We should provide both the source and destination VT for any query like this... should be easy to provide even on the IR path.

Fri, Oct 6, 12:37 PM
efriedma added a comment to D38637: [InstSimplify] don't let poison inhibit an easy fold.

If I understand correctly, the reason computeKnownBits can't handle this is that it doesn't know what to do with a known poison value? We could just solve the issue in computeKnownBits: currently, it says there are no known bits when it detects a shift overflow, but it could just say, for example, that all the bits are known zero (since the result of computeKnownBits is only meaningful if the value isn't poison).

Fri, Oct 6, 12:20 PM
efriedma added a comment to D34515: [ARM] Materialise some boolean values to avoid a branch.

"it" is more like an instruction prefix than an actual instruction from the perspective of the CPU; it gets decoded into the same thing as an ARM mode conditional instruction. It's not an improvement to replace "it" with an actual instruction.

Fri, Oct 6, 11:56 AM

Thu, Oct 5

efriedma added a comment to D38501: [ValueTracking] Fix a misuse of APInt in GetPointerBaseWithConstantOffset.

Well, in theory there's a crash on trunk that can happen without exotic pointers (something like "gep i64, i64* %x, i128 INT128_MIN" will cause an assertion failure in getSExtValue().)

Thu, Oct 5, 2:35 PM
efriedma added a comment to D38501: [ValueTracking] Fix a misuse of APInt in GetPointerBaseWithConstantOffset.

Yes. Isn't that the point of this patch?

Thu, Oct 5, 2:34 PM
efriedma added inline comments to D38501: [ValueTracking] Fix a misuse of APInt in GetPointerBaseWithConstantOffset.
Thu, Oct 5, 1:52 PM
efriedma added inline comments to D34515: [ARM] Materialise some boolean values to avoid a branch.
Thu, Oct 5, 12:53 PM

Wed, Oct 4

efriedma added inline comments to D37343: [CGP] Merge empty case blocks if no extra moves are added..
Wed, Oct 4, 1:53 PM
efriedma accepted D38557: Convert an APInt to int64_t properly in TTI::getGEPCost()..

LGTM

Wed, Oct 4, 1:36 PM
efriedma added inline comments to D38557: Convert an APInt to int64_t properly in TTI::getGEPCost()..
Wed, Oct 4, 1:00 PM
efriedma added a comment to D38499: [BasicAA] Fix adjustToPointerSize in BasicAliasAnalysis.cpp for ptr > 64b.

In my use case, I have 128-bit pointers but "adding" to one of these will only ever modify the lower 64 bits. So for my use case, offsets > 64 bits just don't matter.

Wed, Oct 4, 11:41 AM

Tue, Oct 3

efriedma added a comment to D35635: Optimize {s,u}{add,sub}.with.overflow on ARM.

Is it really necessary to have two different of almost identical code to generate an ARMISD::BRCOND? (I would rather have an explicit check for an AND than two versions of the code, if that's the issue.)

Tue, Oct 3, 5:35 PM
efriedma added inline comments to D37343: [CGP] Merge empty case blocks if no extra moves are added..
Tue, Oct 3, 5:25 PM
efriedma added a comment to D34029: Infer lowest bits of an integer Multiply when the low bits of the operands are known.

Alive is up again.

Tue, Oct 3, 5:05 PM
efriedma closed D38341: [compiler-rt] Add back ARM EABI aliases where legal..

r314851

Tue, Oct 3, 2:27 PM
efriedma committed rL314851: [compiler-rt] Add back ARM EABI aliases where legal..
[compiler-rt] Add back ARM EABI aliases where legal.
Tue, Oct 3, 2:26 PM
efriedma accepted D38262: [AArch64] Use LateSimplifyCFG after expanding atomic operations..

LGTM

Tue, Oct 3, 2:10 PM
efriedma added a comment to D38499: [BasicAA] Fix adjustToPointerSize in BasicAliasAnalysis.cpp for ptr > 64b.

If we switch to using APInt for offsets, adjustToPointerSize() just goes away, so I don't see how this is forward progress.

Tue, Oct 3, 2:08 PM
efriedma accepted D38085: Use the basic cost if a GEP is not used as addressing mode.

Oh, I see, cast<Operator>(U) will crash in cases where the user is something else, like a global variable.

Tue, Oct 3, 2:01 PM
efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Tue, Oct 3, 12:08 PM
efriedma added inline comments to D37460: [GVN] Prevent LoadPRE from hoisting across instructions that don't pass control flow to successors.
Tue, Oct 3, 11:53 AM
efriedma requested changes to D38499: [BasicAA] Fix adjustToPointerSize in BasicAliasAnalysis.cpp for ptr > 64b.

Hang on, there's a more fundamental problem here this is papering over. If your pointers are larger than 64 bits, those pointers can have offsets larger than 64 bits. Since BasicAA is using 64-bit integers to represent pointer offsets, the math in DecomposeGEPExpression will overflow, so you'll get incorrect results, and eventually cause a miscompile.

Tue, Oct 3, 11:40 AM
efriedma added inline comments to D37460: [GVN] Prevent LoadPRE from hoisting across instructions that don't pass control flow to successors.
Tue, Oct 3, 11:24 AM

Mon, Oct 2

efriedma updated the diff for D38341: [compiler-rt] Add back ARM EABI aliases where legal..

Updated to preserve function signatures for EABI functions.

Mon, Oct 2, 7:24 PM
efriedma added a comment to D38479: Make -mgeneral-regs-only more like GCC's.

As far as I can see, there are three significant issues with the current -mgeneral-regs-only:

Mon, Oct 2, 6:52 PM
efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Mon, Oct 2, 2:31 PM
efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Mon, Oct 2, 2:10 PM
efriedma accepted D38233: [InlineCost, NFC] Extract code dealing with inbounds GEPs from CallAnalyzer::visitGetElementPtr into separate function.

LGTM

Mon, Oct 2, 2:02 PM
efriedma added inline comments to D38262: [AArch64] Use LateSimplifyCFG after expanding atomic operations..
Mon, Oct 2, 1:43 PM

Fri, Sep 29

efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Fri, Sep 29, 12:57 PM
efriedma added inline comments to D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible.
Fri, Sep 29, 12:35 PM
efriedma updated subscribers of D37343: [CGP] Merge empty case blocks if no extra moves are added..
Fri, Sep 29, 12:23 PM

Thu, Sep 28

efriedma added inline comments to D37343: [CGP] Merge empty case blocks if no extra moves are added..
Thu, Sep 28, 5:05 PM
efriedma updated subscribers of D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion.

Update: https://reviews.llvm.org/rL314435 has now landed, so we can use domtree to detect dead blocks.

Thu, Sep 28, 12:38 PM
efriedma added a comment to D38341: [compiler-rt] Add back ARM EABI aliases where legal..

These functions would only really compile down to an additional branch.

Thu, Sep 28, 12:22 PM

Wed, Sep 27

efriedma created D38341: [compiler-rt] Add back ARM EABI aliases where legal..
Wed, Sep 27, 5:36 PM
efriedma committed rL314364: [Preprocessor] Preserve #pragma clang assume_nonnull in preprocessed output.
[Preprocessor] Preserve #pragma clang assume_nonnull in preprocessed output
Wed, Sep 27, 4:31 PM
efriedma closed D37861: preserving #pragma clang assume_nonnull in preprocessed output by committing rL314364: [Preprocessor] Preserve #pragma clang assume_nonnull in preprocessed output.
Wed, Sep 27, 4:31 PM
efriedma accepted D38337: Check for overflows when calculating the offset in GetGEPCost..

LGTM

Wed, Sep 27, 4:17 PM
efriedma added inline comments to D38337: Check for overflows when calculating the offset in GetGEPCost..
Wed, Sep 27, 4:04 PM
efriedma added inline comments to D38336: Add an @llvm.sideeffect intrinsic.
Wed, Sep 27, 3:46 PM
efriedma added inline comments to D38337: Check for overflows when calculating the offset in GetGEPCost..
Wed, Sep 27, 3:28 PM
efriedma updated the diff for D38299: [ARM] Honor -mfloat-abi for libcall calling convention.

Fix CHECK lines.

Wed, Sep 27, 2:05 PM
efriedma added a comment to D38316: [InstCombine] replace bitcast to scalar + insertelement with widening shuffle + vector bitcast.

No, you're right, that's still three instructions; I didn't realize there wasn't a ymm version of vpinsrd.

Wed, Sep 27, 1:47 PM
efriedma added a comment to D38316: [InstCombine] replace bitcast to scalar + insertelement with widening shuffle + vector bitcast.

I meant, "how do we fix x86 in the general case"? Consider the following (with -mtriple=x86_64 -mattr=+xop):

Wed, Sep 27, 12:43 PM
efriedma added a comment to D37861: preserving #pragma clang assume_nonnull in preprocessed output.

Oops, sorry, lost track of it; I'll commit it today.

Wed, Sep 27, 12:37 PM
efriedma added a comment to D38316: [InstCombine] replace bitcast to scalar + insertelement with widening shuffle + vector bitcast.

Right - I thought about that case, but I couldn't justify it if we need another IR instruction.

Wed, Sep 27, 11:20 AM
efriedma updated subscribers of D38316: [InstCombine] replace bitcast to scalar + insertelement with widening shuffle + vector bitcast.

ARM/AArch64 are very similar in this respect, since there are multiple vector register sizes. You'll see a similar result for your examples on aarch64. (On 32-bit ARM, we manage to optimize away the extra copy after isel.) I'm not quite sure how much of this logic it makes sense to put into instcombine, given most of the benefit here has to do with the way CPUs split integer and vector registers, but this is probably okay for other targets.

Wed, Sep 27, 10:58 AM

Tue, Sep 26

efriedma updated subscribers of D38303: [Sema] Correct IUnknown to support Unknwnbase.h Header..
Tue, Sep 26, 9:42 PM
efriedma accepted D38138: [SimplifyCFG] add a struct to house optional folds (PR34603).

Yes, this looks right.

Tue, Sep 26, 5:25 PM
efriedma added a comment to D38050: [ARM] Use correct calling convention for libm..

See https://reviews.llvm.org/D38299; from the discussion here and my testing, it seems like that patch more closely matches what gcc actually does.

Tue, Sep 26, 4:28 PM
efriedma created D38299: [ARM] Honor -mfloat-abi for libcall calling convention.
Tue, Sep 26, 4:27 PM
efriedma accepted D36487: Emit section information for extern variables. .

LGTM

Tue, Sep 26, 3:55 PM
efriedma added reviewers for D37343: [CGP] Merge empty case blocks if no extra moves are added.: hfinkel, qcolombet.

The real problem with computing DT there isn't how long it takes in common cases, it's that the total time will blow up quadratically for large functions.

Tue, Sep 26, 3:30 PM

Mon, Sep 25

efriedma committed rL314169: Revert r312724 ("[ARM] Remove redundant vcvt patterns.")..
Revert r312724 ("[ARM] Remove redundant vcvt patterns.").
Mon, Sep 25, 3:09 PM
efriedma added a comment to D38262: [AArch64] Use LateSimplifyCFG after expanding atomic operations..

Looks reasonable... but missing testcase.

Mon, Sep 25, 2:58 PM
efriedma committed rL314163: [ARM] Fix tests for vcvt+store to return void..
[ARM] Fix tests for vcvt+store to return void.
Mon, Sep 25, 2:57 PM
efriedma committed rL314161: [ARM] Add tests for vcvt followed by store..
[ARM] Add tests for vcvt followed by store.
Mon, Sep 25, 2:39 PM
efriedma committed rL314160: [ARM] Regenerate vcvt test checks..
[ARM] Regenerate vcvt test checks.
Mon, Sep 25, 2:36 PM
efriedma accepted D37665: [SelectionDAG] Teach simplifyDemandedBits to handle shifts by constant splat vectors.

LGTM.

Mon, Sep 25, 11:55 AM
efriedma added a comment to D37343: [CGP] Merge empty case blocks if no extra moves are added..

Remove quadratic complexity to calculate DT.

Mon, Sep 25, 11:12 AM
efriedma added a comment to D36487: Emit section information for extern variables. .

Should I restrict the warning to just redeclarations (without definition) instead?

Mon, Sep 25, 5:02 AM

Fri, Sep 22

efriedma updated subscribers of D37898: [TargetLowering] Correctly track NumFixedArgs field of CallLoweringInfo.

x86, arm, and aarch64 have calling conventions which care whether a call is varargs, but not whether a particular argument is specified in the prototype.

Fri, Sep 22, 11:23 AM

Thu, Sep 21

efriedma updated subscribers of D37826: Refine generation of TBAA information in clang.
Thu, Sep 21, 8:08 PM
efriedma updated subscribers of D38126: Make TBAA information to be part of LValueBaseInfo.

Please make sure to add the mailing list as a subscriber when you post a patch. (I haven't looked at the patch yet.)

Thu, Sep 21, 8:07 PM
efriedma accepted D37861: preserving #pragma clang assume_nonnull in preprocessed output.

Do you want me to commit this for you?

Thu, Sep 21, 2:39 PM
efriedma added inline comments to D38154: [PassManager] Run global opts after the inliner.
Thu, Sep 21, 2:35 PM
efriedma accepted D38145: Suppress Wsign-conversion for enums with matching underlying type.

LGTM

Thu, Sep 21, 12:34 PM
efriedma added inline comments to D38145: Suppress Wsign-conversion for enums with matching underlying type.
Thu, Sep 21, 12:27 PM
efriedma accepted D38046: [Atomic][X8664] set max atomic inline/promote width according to the target.

LGTM

Thu, Sep 21, 12:07 PM
efriedma added inline comments to D38145: Suppress Wsign-conversion for enums with matching underlying type.
Thu, Sep 21, 11:45 AM