Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (166 w, 6 d)

Recent Activity

Mon, Oct 21

efriedma accepted D68194: [LCSSA] Forget values we create LCSSA phis for.

LGTM

Mon, Oct 21, 5:48 PM · Restricted Project
efriedma added a comment to D66725: [DAGCombiner][TargetLowering] Target hook for FCOPYSIGN arg cast folding.

For FCOPYSIGN, specifically, LegalizeDAG is going to query the target to ask, "is FCOPYSIGN legal for result type X", using the getOperationAction() API? The target has a few different ways to respond: here, the relevant possibilities are "Legal", "Expand", or "Custom". I guess the issue here is that on RISCV, this query is returning "Legal", when it actually isn't legal for all combinations of legal result/input types?

Mon, Oct 21, 4:05 PM · Restricted Project

Fri, Oct 18

efriedma accepted D68783: [AArch64][DebugInfo] Do not recompute CalleeSavedStackSize (Take 2).

I was considering whether, instead of making getCalleeSavedStackSize approximate the stack size when running an individual pass, we should serialize it in the MIR. But this is probably okay as-is.

Fri, Oct 18, 6:15 PM · Restricted Project
efriedma added a comment to D68281: [LoopDataPrefetch] Don't prefetch past a known total trip count.

For the LSR thing, see https://reviews.llvm.org/D68592 .

Fri, Oct 18, 12:12 PM
efriedma added inline comments to D69013: [AArch64][X86] Don't assume __powidf2 is available on Windows..
Fri, Oct 18, 12:02 PM · Restricted Project
efriedma added a comment to D68281: [LoopDataPrefetch] Don't prefetch past a known total trip count.

Hopefully someone more familiar with LoopDataPrefetch can review, but this looks reasonable.

Fri, Oct 18, 12:02 PM

Thu, Oct 17

efriedma committed rG5b0e039a7a7d: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3 (authored by efriedma).
[ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3
Thu, Oct 17, 2:57 PM
efriedma closed D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3.
Thu, Oct 17, 2:57 PM · Restricted Project
efriedma committed rL375179: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3.
[ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3
Thu, Oct 17, 2:57 PM
efriedma closed D68675: [9.0 branch][ARM] VFPv2 only supports 16 D registers..

(Merged to the branch.)

Thu, Oct 17, 2:57 PM · Restricted Project
efriedma updated the diff for D69013: [AArch64][X86] Don't assume __powidf2 is available on Windows..

Removed redundant check. Added support for x86.

Thu, Oct 17, 2:57 PM · Restricted Project

Wed, Oct 16

efriedma added a comment to D69013: [AArch64][X86] Don't assume __powidf2 is available on Windows..

I do believe that we need it for all the targets (x86, x86_64, armv7, aarch64).

Wed, Oct 16, 4:04 PM · Restricted Project

Tue, Oct 15

efriedma created D69013: [AArch64][X86] Don't assume __powidf2 is available on Windows..
Tue, Oct 15, 5:35 PM · Restricted Project
efriedma created D69012: [Headers] Fix compatibility between arm_acle.h and intrin.h.
Tue, Oct 15, 4:14 PM · Restricted Project
efriedma added a comment to D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3.

Ping

Tue, Oct 15, 4:13 PM · Restricted Project
efriedma added a comment to D68408: [InstCombine] Negator - sink sinkable negations.

If that doesn't happen, then they are 'trivially' dead and deleted.

Tue, Oct 15, 3:10 PM · Restricted Project
efriedma committed rL374939: Add myself to github-usernames.txt.
Add myself to github-usernames.txt
Tue, Oct 15, 11:46 AM
efriedma accepted D67392: [ARM][ParallelDSP] Change smlad insertion order.

LGTM, assuming you fix your tree so test/MC/AsmParser/preserve-comments-crlf.s isn't modified.

Tue, Oct 15, 11:17 AM · Restricted Project
efriedma updated subscribers of D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.
Tue, Oct 15, 11:08 AM · Restricted Project

Mon, Oct 14

efriedma updated subscribers of D68916: [ARM] Accept ldrb.w mnemonic for certain addressing modes (PR43382).

Oh, that makes sense. But needs a comment on the pattern, and in cvtThumb2LDRB_PRE/POST.

Mon, Oct 14, 5:37 PM · Restricted Project
efriedma added a comment to D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls.

Bug2: test case provided above. Please Lewis take a look at how this case can be fixed.

Mon, Oct 14, 4:20 PM · Restricted Project, Restricted Project
efriedma added a comment to D68408: [InstCombine] Negator - sink sinkable negations.

Hmm, that's a bit more complicated than I hoped it would be... but I don't see any obvious way to simplify it.

Mon, Oct 14, 3:50 PM · Restricted Project
efriedma committed rG4498d41932c6: [test] Fix test failure (authored by efriedma).
[test] Fix test failure
Mon, Oct 14, 3:47 PM
efriedma committed rL374836: [test] Fix test failure.
[test] Fix test failure
Mon, Oct 14, 3:47 PM
efriedma closed D68882: [test] Fix test failure.
Mon, Oct 14, 3:47 PM · Restricted Project
efriedma abandoned D68849: [Parse] Don't speculatively parse an identifier in the wrong context..
Mon, Oct 14, 3:28 PM · Restricted Project
efriedma added a comment to D50685: [AArch64] Support conversion between fp16 and fp128.

It looks like this is still waiting for review comments to be addressed.

Mon, Oct 14, 3:10 PM · Restricted Project
efriedma added a comment to D68898: JumpThreading: enhance JT to handle BB with no successor and address comparison.

Handling what Wei's case will be a nice thing to have, but it may require more significant change in JT.

Mon, Oct 14, 1:15 PM
efriedma accepted D68896: PR43080: Do not build context-sensitive expressions during name classification..

I'm inclined to defer doing that for now, if that's OK :)

Mon, Oct 14, 12:18 PM · Restricted Project
efriedma added a comment to D68916: [ARM] Accept ldrb.w mnemonic for certain addressing modes (PR43382).

Adding aliases seems like the right approach; we have aliases for a bunch of other instructions.

Mon, Oct 14, 11:49 AM · Restricted Project

Fri, Oct 11

efriedma added a comment to D68896: PR43080: Do not build context-sensitive expressions during name classification..

Hmm, the ObjC changes were simpler than I expected. And you managed to avoid making changes to overload sets.

Fri, Oct 11, 6:05 PM · Restricted Project
efriedma added a comment to D68898: JumpThreading: enhance JT to handle BB with no successor and address comparison.

If the terminator is a "ret", or some arbitrary terminator that doesn't simplify, it's not really "threading"; it's just tail duplication. That's likely profitable in some cases, but using ThreadEdge to perform the transform seems confusing.

Fri, Oct 11, 5:38 PM

Thu, Oct 10

efriedma created D68849: [Parse] Don't speculatively parse an identifier in the wrong context..
Thu, Oct 10, 6:26 PM · Restricted Project
efriedma updated the diff for D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3.
Thu, Oct 10, 3:49 PM · Restricted Project
efriedma added inline comments to D68720: Support -fstack-clash-protection for x86.
Thu, Oct 10, 3:39 PM · Restricted Project, Restricted Project
efriedma created D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3.
Thu, Oct 10, 3:12 PM · Restricted Project
efriedma committed rG30a96d3fcb76: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2. (authored by efriedma).
[ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.
Thu, Oct 10, 11:51 AM
efriedma closed D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2..
Thu, Oct 10, 11:51 AM · Restricted Project
efriedma committed rL374419: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2..
[ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.
Thu, Oct 10, 11:50 AM
efriedma added a comment to D68748: Add FMF to vector ops for phi.

It looks like llvm-commits wasn't CC'ed, somehow.

Thu, Oct 10, 8:40 AM · Restricted Project

Wed, Oct 9

efriedma updated the diff for D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2..

Found another broken case. Fix, and add more test coverage.

Wed, Oct 9, 6:32 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

The sequence from https://reviews.llvm.org/D67986#1692153 works, I guess.

Wed, Oct 9, 5:55 PM · Restricted Project
efriedma added a comment to D68720: Support -fstack-clash-protection for x86.

(b) is an issue, as pointed out in https://lwn.net/Articles/726587/ (grep for valgrind) : from valgrind point of view, accessing un-allocated stack memory triggers error, and we probably want to please valgrind

Doing the call *after* the stack allocation is also not an option, as a signal could be raised between the stack allocation and the stack probing, escaping the stack probe if a custom signal handler is executed.

Wed, Oct 9, 5:08 PM · Restricted Project, Restricted Project
efriedma created D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2..
Wed, Oct 9, 4:40 PM · Restricted Project
efriedma added a comment to D68720: Support -fstack-clash-protection for x86.

Sorry, I meant the "probe-stack" attribute.

Wed, Oct 9, 12:23 PM · Restricted Project, Restricted Project
efriedma added a comment to D68720: Support -fstack-clash-protection for x86.

Is there some reason this isn't using the existing stack-probe-size attribute?

Wed, Oct 9, 12:23 PM · Restricted Project, Restricted Project
efriedma added inline comments to D67199: [InstCombine] Expand the simplification of log().
Wed, Oct 9, 12:13 PM · Restricted Project
efriedma accepted D68715: [mangle] Fix mangling where an extra mangle context is required..

LGTM

Wed, Oct 9, 12:04 PM · Restricted Project
efriedma accepted D67350: [IfCvt][ARM] Optimise diamond if-conversion for code size.

LGTM

Wed, Oct 9, 11:54 AM · Restricted Project
efriedma added a comment to D68715: [mangle] Fix mangling where an extra mangle context is required..

The refactoring of the getCurrentMangleNumberContext API mixed with functional changes makes this much harder to review quickly. Please separate it out.

Wed, Oct 9, 11:26 AM · Restricted Project
efriedma committed rG4c4df441860c: [ARM] Fix arm_neon.h with -flax-vector-conversions=none (authored by efriedma).
[ARM] Fix arm_neon.h with -flax-vector-conversions=none
Wed, Oct 9, 10:59 AM
efriedma closed D68683: ARM] Fix arm_neon.h with -flax-vector-conversions=none.
Wed, Oct 9, 10:58 AM · Restricted Project
efriedma committed rL374191: [ARM] Fix arm_neon.h with -flax-vector-conversions=none.
[ARM] Fix arm_neon.h with -flax-vector-conversions=none
Wed, Oct 9, 10:58 AM

Tue, Oct 8

efriedma created D68683: ARM] Fix arm_neon.h with -flax-vector-conversions=none.
Tue, Oct 8, 8:31 PM · Restricted Project
efriedma updated subscribers of D66935: [AArch64][DebugInfo] Do not recompute CalleeSavedStackSize.

Thanks for pointing that out.

Tue, Oct 8, 6:00 PM · Restricted Project
efriedma created D68675: [9.0 branch][ARM] VFPv2 only supports 16 D registers..
Tue, Oct 8, 5:32 PM · Restricted Project
efriedma accepted D68257: [Support] Add mathematical constants.

LGTM

Tue, Oct 8, 4:55 PM · Restricted Project

Mon, Oct 7

efriedma added inline comments to D68257: [Support] Add mathematical constants.
Mon, Oct 7, 3:28 PM · Restricted Project
efriedma added a comment to D68194: [LCSSA] Forget values we create LCSSA phis for.

I know this contradicts existing code in SCEV that tries not to break LCSSA, I'm wondering if we should just remove those in favor of teaching SCEV expander about preserving LCSSA.

Mon, Oct 7, 3:04 PM · Restricted Project
efriedma requested changes to D68257: [Support] Add mathematical constants.
Mon, Oct 7, 2:54 PM · Restricted Project
efriedma added a comment to D29121: [Docs] Add LangRef documention for freeze instruction.

we want freeze to be fully agnostic, it should not care *at all* what the type is, right?

Mon, Oct 7, 1:11 PM · Restricted Project
efriedma added a comment to D53876: Preserve loop metadata when splitting exit blocks.

Is this patch still relevant?

Mon, Oct 7, 12:49 PM · Restricted Project
efriedma reopened D53876: Preserve loop metadata when splitting exit blocks.
Mon, Oct 7, 12:48 PM · Restricted Project
efriedma reopened D30471: [SDAG] Relax conditions under stores of loaded values can be merged.
Mon, Oct 7, 12:47 PM · Restricted Project
efriedma planned changes to D32239: [SCEV] Make SCEV or modeling more aggressive..
Mon, Oct 7, 12:21 PM · Restricted Project
efriedma reopened D32239: [SCEV] Make SCEV or modeling more aggressive..

This was reverted.

Mon, Oct 7, 12:21 PM · Restricted Project
efriedma added inline comments to D67749: [AArch64] Stackframe accesses to SVE objects..
Mon, Oct 7, 12:21 PM · Restricted Project
efriedma closed D32239: [SCEV] Make SCEV or modeling more aggressive..
Mon, Oct 7, 6:14 AM · Restricted Project

Fri, Oct 4

efriedma added a comment to D68231: [SLC] Allow llvm.pow(x,2.0) -> x*x etc even if no pow() lib func.

I don't understand what the check is supposed to be doing in the first place. If we're in LibCallSimplifier::optimizePow, we've already checked that the call is actually calling a function with the right semantics, not just the name.

Fri, Oct 4, 4:40 PM · Restricted Project
efriedma added a comment to D68408: [InstCombine] Negator - sink sinkable negations.

This *is* unusual. Is this too ugly to live?

Fri, Oct 4, 2:30 PM · Restricted Project
efriedma accepted D68463: [ARM] Generate vcmp instead of vcmpe.

Formally, whether we lower the "fcmp" instruction using vcmp or vcmpe shouldn't matter; the difference only affects floating point state the code isn't supposed to access anyway.

Fri, Oct 4, 1:08 PM · Restricted Project
efriedma committed rG23ae13d51f23: [ScheduleDAG] When a node is cloned, add an edge between the nodes. (authored by efriedma).
[ScheduleDAG] When a node is cloned, add an edge between the nodes.
Fri, Oct 4, 12:54 PM
efriedma committed rL373782: [ScheduleDAG] When a node is cloned, add an edge between the nodes..
[ScheduleDAG] When a node is cloned, add an edge between the nodes.
Fri, Oct 4, 12:49 PM
efriedma closed D68068: [ScheduleDAG] When a node is cloned, add an edge between the nodes..
Fri, Oct 4, 12:49 PM · Restricted Project

Wed, Oct 2

efriedma added inline comments to D68360: PR41162 Implement LKK remainder and divisibility algorithms [urem].
Wed, Oct 2, 5:05 PM · Restricted Project
efriedma added inline comments to D68257: [Support] Add mathematical constants.
Wed, Oct 2, 4:02 PM · Restricted Project
efriedma added inline comments to D68257: [Support] Add mathematical constants.
Wed, Oct 2, 3:57 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

You could do something like if (!memccpy (d, s, '\0', size - 1)) d[size - 1] = 0;, I guess. That's a few more instructions that I'd really like, but I don't see a better alternative.

Wed, Oct 2, 3:36 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

-fno-builtin-memccpy doesn't work because memccpy isn't listed in include/clang/Basic/Builtins.def. We should probably fix that.

Wed, Oct 2, 1:58 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

Oh, hmm... actually, just realized something; is the new formulation actually equivalent? Specifically, is it okay to write to "d[size - 1]" if the string is short? Say, for example, you have something like snprintf(buf, 10, "%s", "x"). That normally writes to two elements of the array: 0 and 1. The rewritten version writes to three elements: 0, 1, and 9.

Wed, Oct 2, 1:58 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

It sounds like memccpy is part of C20. Can we not do this transform unless LangOpt says we're C20 or greater? Otherwise the Linux kernel doesn't implement this routine.

Wed, Oct 2, 12:40 PM · Restricted Project

Tue, Oct 1

efriedma added inline comments to D68257: [Support] Add mathematical constants.
Tue, Oct 1, 3:38 PM · Restricted Project

Mon, Sep 30

efriedma accepted D61437: [AArch64] Static (de)allocation of SVE stack objects..

I think you've basically landed all the significant pieces of this already? But sure, LGTM.

Mon, Sep 30, 6:31 PM · Restricted Project
efriedma abandoned D54730: [DomTree] Fix order of domtree updates in MergeBlockIntoPredecessor..
Mon, Sep 30, 12:02 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

Yes, that's right. Thanks for spotting that.

Mon, Sep 30, 11:32 AM · Restricted Project

Thu, Sep 26

efriedma added a comment to D68089: [InstCombine] Optimize some memccpy calls to memcpy/null.

We dont even need to know n. If c is not in src, we have just memcpy(dst, src, n), no?

Thu, Sep 26, 4:02 PM · Restricted Project
efriedma added a comment to D68089: [InstCombine] Optimize some memccpy calls to memcpy/null.

(Comments using argument names from the signature void *memccpy(void *dest, const void *src, int c, size_t n);.)

Thu, Sep 26, 2:33 PM · Restricted Project
efriedma accepted D68019: [ARM][CGP] Allow signext arguments.

LGTM

Thu, Sep 26, 1:08 PM · Restricted Project

Wed, Sep 25

efriedma created D68068: [ScheduleDAG] When a node is cloned, add an edge between the nodes..
Wed, Sep 25, 7:00 PM · Restricted Project
efriedma added a comment to D68019: [ARM][CGP] Allow signext arguments.

Obviously ignoring the attribute is correct; signext is just an ABI attribute which has no semantic effect at the IR level. But why was the code to check it added in https://reviews.llvm.org/D61381? I assume it was supposed to be some sort of profitability check.

Wed, Sep 25, 5:17 PM · Restricted Project
efriedma accepted D67832: [IfConversion] Disallow TBB == FBB for valid triangles.

LGTM

Wed, Sep 25, 5:04 PM · Restricted Project
efriedma committed rG69dddfe26836: [LICM] Don't verify domtree/loopinfo unless EXPENSIVE_CHECKS is enabled. (authored by efriedma).
[LICM] Don't verify domtree/loopinfo unless EXPENSIVE_CHECKS is enabled.
Wed, Sep 25, 3:36 PM
efriedma committed rL372924: [LICM] Don't verify domtree/loopinfo unless EXPENSIVE_CHECKS is enabled..
[LICM] Don't verify domtree/loopinfo unless EXPENSIVE_CHECKS is enabled.
Wed, Sep 25, 3:35 PM
efriedma closed D67571: [LICM] Don't verify domtree/loopinfo unless EXPENSIVE_CHECKS is enabled..
Wed, Sep 25, 3:35 PM · Restricted Project

Tue, Sep 24

efriedma updated subscribers of D38479: Make -mgeneral-regs-only more like GCC's.

Does this have any significant impact on -fsyntax-only performance?

Tue, Sep 24, 5:22 PM · Restricted Project
efriedma added a comment to D67986: [InstCombine] snprintf (d, size, "%s", s) -> memccpy (d, s, '\0', size - 1), d[size - 1] = 0.

Do we have accurate TLI information for memccpy? Not sure exactly how widely available it is.

Tue, Sep 24, 4:43 PM · Restricted Project
efriedma added a comment to D67612: [UnrolledInstAnalyzer] Use MSSA to find stored values outside of loop..

I think I see the gap in communication here.

Tue, Sep 24, 4:30 PM · Restricted Project
efriedma added inline comments to D66955: [DebugInfo][If-Converter] Update call site info during the optimization.
Tue, Sep 24, 1:04 PM · Restricted Project, debug-info
efriedma added a comment to D67832: [IfConversion] Disallow TBB == FBB for valid triangles.

Would it make more sense to just say this isn't a triangle? We don't really want to do loop peeling in IfConversion.

Tue, Sep 24, 1:04 PM · Restricted Project
efriedma added a comment to D67956: [ARM] Lower MVE i1 inserts through shifts, not inreg extend.

If SelectionDAGLegalize::ExpandNode is expanding SIGN_EXTEND_INREG of i1 to a non-canonical form, maybe it makes more sense to fix that, instead of trying to avoid that codepath? I mean, this is fine, given we know it's going to get expanded anyway. But the same problem might come up again later out of target-independent code generating an i1 SIGN_EXTEND_INREG.

Tue, Sep 24, 12:49 PM · Restricted Project