efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (45 w, 3 d)

Recent Activity

Fri, Jun 23

efriedma accepted D34311: [InstCombine] Don't replace allocas with smaller globals.
Fri, Jun 23, 6:14 PM
efriedma added a comment to D34311: [InstCombine] Don't replace allocas with smaller globals.

LGTM.

Fri, Jun 23, 6:14 PM
efriedma accepted D34585: Make visible isDereferenceableAndAlignedPointer(..., const APInt &Size, ...).

LGTM.

Fri, Jun 23, 5:22 PM
efriedma added inline comments to D34311: [InstCombine] Don't replace allocas with smaller globals.
Fri, Jun 23, 5:22 PM
efriedma added inline comments to D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode..
Fri, Jun 23, 5:09 PM
efriedma added a comment to D34523: AST: mangle BlockDecls under MS ABI.
How do we end up in a state where the block invocation function is referenced external to the TU?
Fri, Jun 23, 3:09 PM · Restricted Project
efriedma added a comment to D34523: AST: mangle BlockDecls under MS ABI.

I think we just use "0" as a sentinel value to indicate the block doesn't need a mangling number. getLambdaManglingNumber works the same way? See CXXNameMangler::mangleUnqualifiedBlock in the Itanium mangler.

Fri, Jun 23, 2:06 PM · Restricted Project
efriedma added inline comments to D34538: Handle ConstantExpr correctly in SelectionDAGBuilder.
Fri, Jun 23, 1:43 PM
efriedma created D34568: [Sema] Make BreakContinueFinder handle nested loops..
Fri, Jun 23, 1:09 PM
efriedma added a comment to D34538: Handle ConstantExpr correctly in SelectionDAGBuilder.

I'll think about the SelectionDAGBuilder::visit issue myself, and maybe come up with a followup patch.

Fri, Jun 23, 12:53 PM

Thu, Jun 22

efriedma updated subscribers of D34458: [TTI] Refine the cost of EXT in getUserCost().
Thu, Jun 22, 3:37 PM
efriedma edited reviewers for D34538: Handle ConstantExpr correctly in SelectionDAGBuilder, added: efriedma; removed: llvm-commits.

Missing testcase... although, I'm not how you would write one.

Thu, Jun 22, 3:25 PM
efriedma added a comment to D34523: AST: mangle BlockDecls under MS ABI.

@efriedma hmm...using getBlockManglingNumber causes the name to be duplicated. Ill look into that.

Thu, Jun 22, 2:58 PM · Restricted Project
efriedma added a comment to D34471: [Inliner] Boost inlining of an indirect call to always_inline function..

I think this is a fairly uncommon scenario that size increase is not a concern in general,

Thu, Jun 22, 2:33 PM
efriedma accepted D34516: [LoopDeletion] Update exits correctly when multiple duplicate edges from an exiting block.

LGTM with the comment fixed.

Thu, Jun 22, 12:56 PM
efriedma added inline comments to D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands..
Thu, Jun 22, 12:53 PM
efriedma updated subscribers of D34515: [ARM] Materialise some boolean values to avoid a branch.

A lot of these combines look very similar to the work done for ISD::ADDCARRY/SUBCARRY; see https://reviews.llvm.org/D29872 . Can we reuse that work rather than add a bunch of ARM-specific code?

Thu, Jun 22, 12:36 PM
efriedma added a comment to D34311: [InstCombine] Don't replace allocas with smaller globals.

There's a isDereferenceableAndAlignedPointer helper which does this sort of check. See also https://reviews.llvm.org/D34467.

Thu, Jun 22, 12:30 PM
efriedma added inline comments to D34516: [LoopDeletion] Update exits correctly when multiple duplicate edges from an exiting block.
Thu, Jun 22, 12:27 PM
efriedma added a comment to D34523: AST: mangle BlockDecls under MS ABI.

Patch is missing context.

Thu, Jun 22, 12:14 PM · Restricted Project
efriedma added a comment to D34471: [Inliner] Boost inlining of an indirect call to always_inline function..

This seems likely to lead to surprising behavior. LastCallToStaticBonus is very large, so we'll end up duplicating a lot of code in some cases, which could be surprising. On the other hand, it isn't infinitely large, so you can't depend on always_inline to trigger the behavior reliably.

Thu, Jun 22, 11:42 AM
efriedma added a comment to D34518: [ADT] Add llvm::to_float.

it looks like it doesn't easily support long double

Thu, Jun 22, 11:34 AM

Tue, Jun 20

efriedma committed rL305864: [ScopInfo] Fix crash with sum of invariant load and AddRec..
[ScopInfo] Fix crash with sum of invariant load and AddRec.
Tue, Jun 20, 3:54 PM
efriedma closed D34259: [polly] [ScopInfo] Fix crash with sum of invariant load and AddRec. by committing rL305864: [ScopInfo] Fix crash with sum of invariant load and AddRec..
Tue, Jun 20, 3:53 PM
efriedma added a comment to D34057: [TargetTransformInfo, API] Add optional list of operands to TTI::getUserCost.

Yes, this is what I was meant; thanks.

Tue, Jun 20, 3:27 PM
efriedma added a comment to D34311: [InstCombine] Don't replace allocas with smaller globals.

If we get to "memcpy(z, y, 10);" without "memcpy(y, x, 10);" I'd expect we don't care if "y" is uninitialized bytes or global constant. We will have no buffer overflow which I am trying to fix.

Tue, Jun 20, 3:17 PM
efriedma added a comment to D32720: [LICM] Introduce a finer granularity option to compute early exits..

AssertingVH catching a bug. Cool. :)

Tue, Jun 20, 2:06 PM
efriedma added a comment to D34311: [InstCombine] Don't replace allocas with smaller globals.

Hmm, actually, thinking about it a bit more, I'm not sure this patch is right.

Tue, Jun 20, 12:10 PM

Mon, Jun 19

efriedma added a comment to D34368: [AArch64] PointerRegClass should be GPR64spRegClass.

Testcase? (I think this affects inline asm.)

Mon, Jun 19, 3:23 PM
efriedma added a comment to D33818: [ScheduleDAG] Don't schedule node with physical register interference.

Ping.

Mon, Jun 19, 2:07 PM
efriedma added inline comments to D32720: [LICM] Introduce a finer granularity option to compute early exits..
Mon, Jun 19, 2:01 PM
efriedma accepted D33342: [InstCombine] try to canonicalize xor-of-icmps to and-of-icmps.

Sorry about the delayed review.

Mon, Jun 19, 1:39 PM
efriedma added a reviewer for D34311: [InstCombine] Don't replace allocas with smaller globals: efriedma.

I wonder if it makes sense to more aggressively transform the IR in certain cases rather than bail out, since we can prove that the IR is reading uninitialized memory. Something for a future patch, I guess, if someone has a testcase where it matters in practice.

Mon, Jun 19, 12:44 PM
efriedma added a comment to D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands..

I just tried your updated patch, and I'm seeing crashes in make check. (Assertion Ret.getOpcode() == ISD::MERGE_VALUES && "Ret is merged value node comprising constituents stack" "locations holding result"' failed.`)

Mon, Jun 19, 12:25 PM

Fri, Jun 16

efriedma added a reviewer for D34293: [PATCH] [PGO] Fixed cast operation in emIntrinsicVisitor::instrumentOneMemIntrinsic.: davidxl.
Fri, Jun 16, 2:11 PM
efriedma added inline comments to D34181: MachineInstr: Reason locally about some memory objects before going to AA..
Fri, Jun 16, 1:54 PM
efriedma added a reviewer for D34181: MachineInstr: Reason locally about some memory objects before going to AA.: arsenm.

Any idea what effect this has on performance, if any?

Fri, Jun 16, 1:09 PM
efriedma added a comment to D34057: [TargetTransformInfo, API] Add optional list of operands to TTI::getUserCost.

Umm, that misses the point of my comment; we want to reduce the number of paths through the code by requiring that Operands is never empty (except for operations which don't have any operands).

Fri, Jun 16, 12:38 PM
efriedma added a comment to D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands..

I apologize, my review last night wasn't very insightful. Hopefully better comments today.

Fri, Jun 16, 12:27 PM

Thu, Jun 15

efriedma edited reviewers for D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands., added: efriedma; removed: eli.friedman.

Please make sure to put llvm-commits in the subscriber list when you post a patch.

Thu, Jun 15, 9:43 PM
efriedma created D34259: [polly] [ScopInfo] Fix crash with sum of invariant load and AddRec..
Thu, Jun 15, 4:12 PM

Wed, Jun 14

efriedma added inline comments to D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection.
Wed, Jun 14, 4:11 PM
efriedma added inline comments to D34057: [TargetTransformInfo, API] Add optional list of operands to TTI::getUserCost.
Wed, Jun 14, 3:55 PM
efriedma closed D34188: Don't check side effects for functions outside of SCoP.

https://reviews.llvm.org/rL305423

Wed, Jun 14, 3:45 PM · Restricted Project
efriedma committed rL305423: Don't check side effects for functions outside of SCoP.
Don't check side effects for functions outside of SCoP
Wed, Jun 14, 3:44 PM
efriedma added a comment to D21720: Unroll for uncountable loops.

Changing it in this patch would be considered as "unrelated change".

Wed, Jun 14, 3:32 PM
efriedma added a comment to D34143: Handling of TRAP during isel.

Needs testcase. (-verify-machineinstrs will run the verifier even if it's off by default for your target.)

Wed, Jun 14, 2:39 PM
efriedma added a comment to D34188: Don't check side effects for functions outside of SCoP.

mayReadOrWriteMemory() just checks for the "readnone" attribute (http://llvm.org/docs/LangRef.html#function-attributes). If it's set, LLVM knows the call can't read from or write to memory; if it isn't set, the call might write to memory, so mayReadOrWriteMemory() conservatively returns true.

Wed, Jun 14, 2:16 PM · Restricted Project
efriedma added a comment to D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion.

Can't you just tell LVI the blocks are dead without them being dead?

Wed, Jun 14, 1:12 PM
efriedma added inline comments to D33983: update add\sub costs of vectors of 64 in X86\SLM arch.
Wed, Jun 14, 12:30 PM
efriedma added a comment to D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion.

If the issue in JumpThreading is determining reachability at all times

Wed, Jun 14, 12:03 PM

Tue, Jun 13

efriedma added inline comments to D32720: [LICM] Introduce a finer granularity option to compute early exits..
Tue, Jun 13, 2:48 PM
efriedma added inline comments to D32720: [LICM] Introduce a finer granularity option to compute early exits..
Tue, Jun 13, 2:01 PM
efriedma edited reviewers for D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion, added: efriedma, dberlin; removed: eli.friedman.
Tue, Jun 13, 1:18 PM

Mon, Jun 12

efriedma added a comment to D34085: [PGO] Register promote profile counter updates.

Did you do an analysis of whether this produces the same counter values for single-threaded code?

Mon, Jun 12, 6:07 PM
efriedma added a comment to D34121: [ubsan] Teach the pointer overflow check that "p - <unsigned> <= p" (PR33430).

Looks okay, but I haven't been reviewing this series of patches closely, so I could be missing something.

Mon, Jun 12, 3:33 PM
efriedma added a comment to D34117: PR32632 Add a case to remove loop compare.

I'm not convinced it makes sense to invest any effort into loops with a backedge-taken count of zero. Yes, maybe we simplify the exit condition slightly earlier, but does that actually do us any good in practice?

Mon, Jun 12, 3:13 PM
efriedma requested changes to D34117: PR32632 Add a case to remove loop compare.

Scalar evolution can already analyze the given testcase on trunk. Output of "opt -analyze -scalar-evolution":

Mon, Jun 12, 1:05 PM
efriedma added inline comments to D33910: [ubsan] Detect invalid unsigned pointer index expression (clang).
Mon, Jun 12, 12:34 PM
efriedma added inline comments to D34077: DAGCombine: Combine BUILD_VECTOR to TRUNCATE.
Mon, Jun 12, 12:27 PM

Fri, Jun 9

efriedma added a comment to D33773: [ARM] llc -arm-execute-only with floating point runs into UNREACHABLE.

Custom-lowering the constant pool to a global isn't that terrible, I guess... it's ugly, and we can't share the globals across basic blocks, but it should behave correctly, as far as I know.

Fri, Jun 9, 11:17 AM

Thu, Jun 8

efriedma accepted D34043: [CGP] don't expand a memcmp with nobuiltin attribute.

LGTM with one minor tweak.

Thu, Jun 8, 11:40 AM

Wed, Jun 7

efriedma added inline comments to D33818: [ScheduleDAG] Don't schedule node with physical register interference.
Wed, Jun 7, 4:44 PM
efriedma accepted D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Oh, sorry, about the delay, I lost track of this.

Wed, Jun 7, 2:50 PM
efriedma accepted D33879: [InstCombine] fold lshr (sext X), C1 --> zext (lshr X, C2).

LGTM.

Wed, Jun 7, 12:27 PM
efriedma added a comment to D33818: [ScheduleDAG] Don't schedule node with physical register interference.

Ping.

Wed, Jun 7, 11:23 AM
efriedma accepted D33737: [InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin.

LGTM.

Wed, Jun 7, 11:20 AM
efriedma added inline comments to D33879: [InstCombine] fold lshr (sext X), C1 --> zext (lshr X, C2).
Wed, Jun 7, 11:18 AM

Tue, Jun 6

efriedma accepted D33963: [CGP / PowerPC] use direct compares if there's only one load per block in memcmp() expansion.

LGTM. (We could match this in DAGCombine, but probably not worth bothering.)

Tue, Jun 6, 4:27 PM
efriedma added inline comments to D33963: [CGP / PowerPC] use direct compares if there's only one load per block in memcmp() expansion.
Tue, Jun 6, 4:16 PM
efriedma added a comment to D31972: Do not force the frame pointer by default for ARM EABI.

Please start a thread on cfe-dev about this; most developers don't read cfe-commits, and thinking about it a bit more, I'm not confident omitting frame pointers is really a good default. I would guess there's existing code which depends on frame pointers to come up with a stack trace, since table-based unwinding is complicated and takes up a lot of space.

Tue, Jun 6, 2:36 PM
efriedma added a comment to D33773: [ARM] llc -arm-execute-only with floating point runs into UNREACHABLE.

Most targets keep the constants in the MachineConstantPool, and emit them with AsmPrinter::EmitConstantPool. I would prefer to align with the way other targets handle constant pools, if it isn't too hard.

Tue, Jun 6, 1:58 PM
efriedma added a comment to D33939: Make memory operands for MIs resulting from Memcpy/Memset on SystemZ.

We don't current support MemIntrinsicSDNodes with more than one memory operand at the moment... but I don't think that would be hard to change. Probably worth fixing if you need it.

Tue, Jun 6, 1:09 PM
efriedma accepted D33926: [clang] Remove double semicolons. NFC..

You don't need to ask for review for a trivial change like this.

Tue, Jun 6, 12:19 PM · Restricted Project

Mon, Jun 5

efriedma added inline comments to D33919: [ADT] Make iterable SmallVector template overrides more specific.
Mon, Jun 5, 4:34 PM
efriedma added a comment to D32998: [SROA] enable splitting for non-whole-alloca loads and stores.

I'd like to see a response from Chandler, since he was the last one to touch this code.

Mon, Jun 5, 4:00 PM
efriedma updated subscribers of D33863: [DAGComine] (fadd x, undef) -> undef and (fmul x, undef) -> undef.

fadd NaN, undef -> undef isn't legal: any fadd involving NaN always produces a NaN. Similar reasoning applies to most other floating-point values.

Mon, Jun 5, 12:10 PM
efriedma added a comment to D33866: [DAGCombiner] loosen restriction for creating narrow vector load from extract(wide load).

The diffs to the ARM tests are clearly no good: you're splitting 128-bit vector loads into two 64-bit vector loads for no benefit.

Mon, Jun 5, 11:52 AM

Thu, Jun 1

efriedma created D33818: [ScheduleDAG] Don't schedule node with physical register interference.
Thu, Jun 1, 6:28 PM
efriedma accepted D33795: [PredicateInfo] Enable -reverse-iterate tests only for +Asserts builds.

LGTM

Thu, Jun 1, 4:01 PM
efriedma updated subscribers of D31972: Do not force the frame pointer by default for ARM EABI.
Thu, Jun 1, 3:41 PM
efriedma added inline comments to D33800: [SelectionDAG] Update the dominator after splitting critical edges.
Thu, Jun 1, 2:46 PM
efriedma committed rL304480: Add opt-bisect support to polly..
Add opt-bisect support to polly.
Thu, Jun 1, 2:29 PM
efriedma closed D33752: Add opt-bisect support to polly by committing rL304480: Add opt-bisect support to polly..
Thu, Jun 1, 2:29 PM
efriedma committed rL304476: Add opt-bisect support for region passes..
Add opt-bisect support for region passes.
Thu, Jun 1, 2:23 PM
efriedma closed D33751: Add opt-bisect support for region passes by committing rL304476: Add opt-bisect support for region passes..
Thu, Jun 1, 2:22 PM
efriedma accepted D33795: [PredicateInfo] Enable -reverse-iterate tests only for +Asserts builds.

Looks fine, but please fix the commit message to note that -reverse-iterate only exists in +Asserts builds.

Thu, Jun 1, 1:40 PM
efriedma added inline comments to D31972: Do not force the frame pointer by default for ARM EABI.
Thu, Jun 1, 12:10 PM
efriedma added a comment to D33773: [ARM] llc -arm-execute-only with floating point runs into UNREACHABLE.

There are a bunch of places in SelectionDAG which generate constant pools for obscure constructs. Maybe we need a separate TargetLowering hook to suppress constant pools in general, rather than just suppressing this particular DAGCombine? (grep for getConstantPool in lib/CodeGen/SelectionDAG/ .)

Thu, Jun 1, 11:43 AM
efriedma accepted D33779: [InlineCost] Add a test case for GEP cost.

LGTM. Thanks!

Thu, Jun 1, 10:55 AM
efriedma added a comment to D33765: Show correct column nr. when multi-byte utf8 chars are used..

Correctly counting columns is a bit more complicated that that... for example, consider what happens if you replace ideëen with idez̈en. See https://stackoverflow.com/questions/3634627/how-to-know-the-preferred-display-width-in-columns-of-unicode-characters .

Thu, Jun 1, 10:48 AM
efriedma added a comment to D33774: [CodeGen] Make __attribute__(const) calls speculatable.

Consider something like this:

Thu, Jun 1, 10:30 AM

Wed, May 31

efriedma added a dependent revision for D33751: Add opt-bisect support for region passes: D33752: Add opt-bisect support to polly.
Wed, May 31, 6:46 PM
efriedma created D33752: Add opt-bisect support to polly.
Wed, May 31, 6:46 PM
efriedma created D33751: Add opt-bisect support for region passes.
Wed, May 31, 6:36 PM
efriedma added inline comments to D33737: [InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin.
Wed, May 31, 4:22 PM
efriedma committed rL304370: [docs] Update name of vectorization interleave flag..
[docs] Update name of vectorization interleave flag.
Wed, May 31, 4:03 PM
efriedma added a comment to D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection.

Okay, in that case the compile-time sounds fine.

Wed, May 31, 3:57 PM
efriedma added a comment to D33737: [InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin.

The other alternative is that you could pass in an AttributeList (the result of getAttributes()) to the functions... that makes it more clear what exactly is getting used, but it's more complicated (and probably involves a bunch of refactoring to expose a function to check isNoBuiltin given a Function/AttributeList pair).

Wed, May 31, 3:29 PM
efriedma edited reviewers for D33737: [InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin, added: efriedma; removed: eli.friedman.

Would it be possible to pass in a CallSite to canConstantFoldCallTo, ConstantFoldCall, and SimplifyCall, to reduce the number of places we check this?

Wed, May 31, 1:37 PM