Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Fri, Aug 16

efriedma added a comment to D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes.

in the proper order

Fri, Aug 16, 5:46 PM · Restricted Project
efriedma committed rGeaff844fe958: [ARM] Preserve liveness in ARMConstantIslands. (authored by efriedma).
[ARM] Preserve liveness in ARMConstantIslands.
Fri, Aug 16, 3:23 PM
efriedma committed rL369162: [ARM] Preserve liveness in ARMConstantIslands..
[ARM] Preserve liveness in ARMConstantIslands.
Fri, Aug 16, 3:23 PM
efriedma closed D66319: [ARM] Preserve liveness in ARMConstantIslands..
Fri, Aug 16, 3:23 PM · Restricted Project
efriedma added inline comments to D66210: [RFC/WIP][RISCV] Enable the machine outliner for RISC-V.
Fri, Aug 16, 1:39 PM · Restricted Project
efriedma updated the diff for D66319: [ARM] Preserve liveness in ARMConstantIslands..

Address review comments.

Fri, Aug 16, 11:40 AM · Restricted Project
efriedma added inline comments to D66319: [ARM] Preserve liveness in ARMConstantIslands..
Fri, Aug 16, 11:40 AM · Restricted Project

Thu, Aug 15

efriedma committed rG9b9a3084521b: [ARM][LowOverheadLoops] Fix generated code for "revert". (authored by efriedma).
[ARM][LowOverheadLoops] Fix generated code for "revert".
Thu, Aug 15, 4:36 PM
efriedma committed rL369069: [ARM][LowOverheadLoops] Fix generated code for "revert"..
[ARM][LowOverheadLoops] Fix generated code for "revert".
Thu, Aug 15, 4:36 PM
efriedma closed D66243: [ARM][LowOverheadLoops] Fix generated code for "revert"..
Thu, Aug 15, 4:36 PM · Restricted Project
efriedma added a comment to D66243: [ARM][LowOverheadLoops] Fix generated code for "revert"..

Posted the liveness patch: https://reviews.llvm.org/D66319

Thu, Aug 15, 4:31 PM · Restricted Project
efriedma created D66319: [ARM] Preserve liveness in ARMConstantIslands..
Thu, Aug 15, 4:22 PM · Restricted Project
efriedma added inline comments to D65020: [GVN] Do PHI translations across all edges between the load and the unavailable pred..
Thu, Aug 15, 12:28 PM · Restricted Project

Wed, Aug 14

efriedma accepted D65653: [AArch64] Change location of frame-record within callee-save area..

LGTM

Wed, Aug 14, 4:24 PM · Restricted Project
efriedma added a comment to D65020: [GVN] Do PHI translations across all edges between the load and the unavailable pred..

Given that each block only has a single predecessor, there aren't any PHI nodes involved. In that case, what does PHITranslateWithInsertion actually do?

Wed, Aug 14, 3:10 PM · Restricted Project
efriedma created D66243: [ARM][LowOverheadLoops] Fix generated code for "revert"..
Wed, Aug 14, 12:54 PM · Restricted Project

Tue, Aug 13

efriedma added inline comments to D65653: [AArch64] Change location of frame-record within callee-save area..
Tue, Aug 13, 4:45 PM · Restricted Project
efriedma added a comment to D63676: Early exit from Hoist() in machine licm pass based on block hotness.

In the future, please try to ping more frequently than once a month; we don't want to lose patches, even if there are some areas where it takes a while to get review.

Tue, Aug 13, 4:32 PM · Restricted Project
efriedma committed rGb5eb3e1e8271: [AArch64] Remove incorrect usage of MONonTemporal. (authored by efriedma).
[AArch64] Remove incorrect usage of MONonTemporal.
Tue, Aug 13, 4:14 PM
efriedma committed rL368770: [AArch64] Remove incorrect usage of MONonTemporal..
[AArch64] Remove incorrect usage of MONonTemporal.
Tue, Aug 13, 4:14 PM
efriedma added a comment to D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes.

If variable A's initializer references variable B, then it will call B's initializer.

Tue, Aug 13, 9:02 AM · Restricted Project

Mon, Aug 12

efriedma added a comment to D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes.

This might be a silly question, but what happens if the initializer for a thread-local variable refers to another thread-local variable? Do you need to initialize both variables? In what order?

Mon, Aug 12, 7:08 PM · Restricted Project
efriedma added a comment to D66114: [ConstantHoisting] Fix non-determinism..

The keys are subclasses of llvm::Value, so sorting is hard. There's no obvious comparator to sort global variables. Instructions and basic blocks could be sorted based on DFS numbers, or something like that, but it would be a lot more complicated.

Mon, Aug 12, 4:05 PM · Restricted Project
efriedma created D66114: [ConstantHoisting] Fix non-determinism..
Mon, Aug 12, 3:17 PM · Restricted Project
efriedma accepted D65234: [CodeGen]: don't treat structures returned in registers as memory inputs.

LGTM

Mon, Aug 12, 2:35 PM · Restricted Project
efriedma added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Mon, Aug 12, 1:32 PM · Restricted Project, Restricted Project
efriedma added a comment to D65019: [ARM] push LR before __gnu_mcount_nc.

It seems global-isel does not fall back to DAGISel?

Mon, Aug 12, 1:04 PM · Restricted Project, Restricted Project

Thu, Aug 8

efriedma added inline comments to D65795: [TargetLowering] Remove optional arguments passing to makeLibCall.
Thu, Aug 8, 3:59 PM · Restricted Project
efriedma added a comment to D65979: [InstCombine] Simplify pow(2.0, itofp(y)) to ldexp(1.0, y).

Does this require fast math due to rounding in the sitofp conversion? Off the top of my head, probably it doesn't matter because those cases would produce zero or infinity anyway, but it's worth explaining in the code.

Thu, Aug 8, 3:16 PM · Restricted Project
efriedma added a comment to D65802: [DAGCombiner] Rebuild (setcc x, y, ==) from (xor (xor x, y), 1).

I don't think this is legal in general unless you restrict it to i1. See https://rise4fun.com/Alive/1Hyj .

Thu, Aug 8, 3:02 PM · Restricted Project
efriedma accepted D65740: [ARM][ParallelDSP] Replace SExt uses.

Okay, makes sense. LGTM

Thu, Aug 8, 1:16 PM · Restricted Project
efriedma added a comment to D65019: [ARM] push LR before __gnu_mcount_nc.

Yes, it's technically a "call", but you don't need the call lowering code. That's dedicated to stuff like passing arguments, returning values, checking whether the function can be tail-called, etc. None of that applies here; the intrinsic always corresponds to exactly one pseudo-instruction, a BL_PUSHLR.

Thu, Aug 8, 12:45 PM · Restricted Project, Restricted Project

Wed, Aug 7

efriedma added a comment to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

The approach seems better; I'll comment in more detail on the other review.

Wed, Aug 7, 3:19 PM · Restricted Project
efriedma added a comment to D65019: [ARM] push LR before __gnu_mcount_nc.

This seems better.

Wed, Aug 7, 11:49 AM · Restricted Project, Restricted Project
efriedma added inline comments to D65718: [LangRef] Document forward-progress requirement.
Wed, Aug 7, 10:49 AM · Restricted Project

Tue, Aug 6

efriedma accepted D65817: [AArch64][WinCFI] Do not pair callee-save instructions in LoadStoreOptimizer.

LGTM

Tue, Aug 6, 12:17 PM · Restricted Project
efriedma accepted D65801: [NFC] Add tests for boolean comparisons.

LGTM

Tue, Aug 6, 11:16 AM · Restricted Project

Mon, Aug 5

efriedma added reviewers for D65740: [ARM][ParallelDSP] Replace SExt uses: SjoerdMeijer, dmgreen.

How did we end up with this testing gap? I haven't been closely watching what tests were committed with previous patches.

Mon, Aug 5, 4:49 PM · Restricted Project
efriedma added a comment to D65019: [ARM] push LR before __gnu_mcount_nc.

introduce quite some code to the target-independent part of SelectionDAG, specifically SelectionDAGBuilder and SelectionDAG classes

Mon, Aug 5, 4:06 PM · Restricted Project, Restricted Project
efriedma added inline comments to D65653: [AArch64] Change location of frame-record within callee-save area..
Mon, Aug 5, 3:20 PM · Restricted Project
efriedma added a comment to D65234: [CodeGen]: don't treat structures returned in registers as memory inputs.

For the "=x" thing I was talking about, try the following testcase:

Mon, Aug 5, 1:50 PM · Restricted Project

Fri, Aug 2

efriedma accepted D65606: [ARM] Fix invalid symbol redefinition due to duplicated jumptable (PR42760).

LGTM

Fri, Aug 2, 2:43 PM · Restricted Project
efriedma added inline comments to D65606: [ARM] Fix invalid symbol redefinition due to duplicated jumptable (PR42760).
Fri, Aug 2, 2:30 PM · Restricted Project
efriedma added a comment to D47927: [TargetLowering] Simplify expansion of S{ADD,SUB}O.

The changes for vector arithmetic look nice.

Fri, Aug 2, 2:19 PM · Restricted Project
efriedma added a comment to D65653: [AArch64] Change location of frame-record within callee-save area..

I can't think of any issue this would cause, in general; following frame records should still work the same way, and DWARF unwinding should still work.

Fri, Aug 2, 12:07 PM · Restricted Project
efriedma added a comment to D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.

Can you remove the dead MachineDominatorTree *MDT; declaration?

Fri, Aug 2, 11:47 AM · Restricted Project
efriedma added a comment to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

I think this is semantically the correct fix, but I have some concern about the way the APIs are written.

Fri, Aug 2, 11:35 AM · Restricted Project
efriedma accepted D65635: Sidestep false positive due to a matching git repository name.

It would be cleaner to convert this test to FileCheck, but it's probably not worth spending the time at this point.

Fri, Aug 2, 10:42 AM · Restricted Project, Restricted Project

Thu, Aug 1

efriedma added a comment to D60582: [IPSCCP] Add general integer range support..

Sorry about the delayed response.

Thu, Aug 1, 4:31 PM · Restricted Project
efriedma added a comment to D65606: [ARM] Fix invalid symbol redefinition due to duplicated jumptable (PR42760).

Maybe you can make a simpler MIR testcase? (http://llvm.org/docs/MIRLangRef.html)

Thu, Aug 1, 2:08 PM · Restricted Project
efriedma added a comment to D65550: [AArch64] Do not emit '#' before immediates in inline asm.

This only affects inline asm, it looks like, not general instructions? (I looked briefly, and it looks like instructions go through AArch64InstPrinter::printOperand instead.)

Thu, Aug 1, 11:54 AM · Restricted Project
efriedma added inline comments to D63782: [FPEnv] Add fptosi and fptoui constrained intrinsics.
Thu, Aug 1, 11:37 AM · Restricted Project

Wed, Jul 31

efriedma committed rG89b80f1239e2: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1. (authored by efriedma).
[ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1.
Wed, Jul 31, 4:20 PM
efriedma committed rL367492: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1..
[ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1.
Wed, Jul 31, 4:20 PM
efriedma closed D65351: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1..
Wed, Jul 31, 4:19 PM · Restricted Project
efriedma committed rG2f45ec1c39dc: [ARM] Transform compare of masked value to shift on Thumb1. (authored by efriedma).
[ARM] Transform compare of masked value to shift on Thumb1.
Wed, Jul 31, 4:19 PM
efriedma committed rL367491: [ARM] Transform compare of masked value to shift on Thumb1..
[ARM] Transform compare of masked value to shift on Thumb1.
Wed, Jul 31, 4:19 PM
efriedma closed D65175: [ARM] Transform compare of masked value to shift on Thumb1..
Wed, Jul 31, 4:19 PM · Restricted Project
efriedma added a comment to D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.

Okay, delaying some of the work to sense, since you can't really modify FunctionChain while you're iterating over it.

Wed, Jul 31, 11:19 AM · Restricted Project
efriedma added a comment to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

I don't think there's any way to define shouldSignExtendTypeInLibCall that completely solves the issue without target-independent changes. Yes, it's true that floats should not be zero-extended, but they also shouldn't be sign-extended. So makeLibCall should do something equivalent to .setSExtResult(false).setZExtResult(false) for the result, and Entry.IsSExt = false; Entry.IsZExt = false; for the arguments. And this needs to apply only to float arguments/results; integer arguments and results need to be sign-extended from 32 bits to 64 bits. (Not sure we currently generate any libcalls with 32-bit arguments/results on RV64, but that could change in the future.)

Wed, Jul 31, 11:03 AM · Restricted Project
efriedma added a comment to D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion.

this looks like it could be a Core Issue

Wed, Jul 31, 10:46 AM · Restricted Project

Tue, Jul 30

efriedma added a comment to D65351: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1..

Would this also be profitable for Thumb2?

Tue, Jul 30, 4:33 PM · Restricted Project
efriedma updated the diff for D65175: [ARM] Transform compare of masked value to shift on Thumb1..

Updated heuristic to match the underlying encodings a bit more closely.

Tue, Jul 30, 4:24 PM · Restricted Project
efriedma added inline comments to D64859: [InstCombine] strncmp(x,y,strlen(x|y)+1) -> strcmp(x,y).
Tue, Jul 30, 2:23 PM · Restricted Project
efriedma accepted D65439: [IPSCCP] Move callsite check to the beginning of the loop..

LGTM

Tue, Jul 30, 12:41 PM · Restricted Project
efriedma added a comment to D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.

Here, if I use F->erase(TBB), the memory leak error is still existed.

Tue, Jul 30, 12:30 PM · Restricted Project
efriedma accepted D65324: [ARM][ParallelDSP] Convert to function pass.

LGTM

Tue, Jul 30, 12:09 PM · Restricted Project
efriedma added a comment to D65457: [InstCombine] Optimize div/ldiv/lldiv.

If we're going to recognize a C function that returns an aggregate, we need to be very careful that the form we're recognizing is a specific, known pattern. We need to be sure that we're recognizing a specific way of returning an aggregate, and TLI needs an API to tell transforms which specific way it recognized. We don't need to add all the possible patterns in the first patch, of course.

Tue, Jul 30, 12:01 PM · Restricted Project

Mon, Jul 29

efriedma reopened D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.

You probably want F->erase(TBB), which both removes TBB from the list of blocks in the function, and deallocates TBB. I guess the empty block with no predecessors doesn't really matter much, in the long run, but easier to understand if the transform cleans up after itself properly.

Mon, Jul 29, 12:40 PM · Restricted Project
efriedma accepted D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins.

LGTM

Mon, Jul 29, 12:38 PM · Restricted Project, Restricted Project
efriedma added inline comments to D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins.
Mon, Jul 29, 12:10 PM · Restricted Project, Restricted Project

Fri, Jul 26

efriedma created D65351: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1..
Fri, Jul 26, 2:49 PM · Restricted Project
efriedma added a child revision for D65175: [ARM] Transform compare of masked value to shift on Thumb1.: D65351: [ARM] Lower "(x<<c) > 0x80000000U" to "lsls" on Thumb1..
Fri, Jul 26, 2:49 PM · Restricted Project
efriedma accepted D65310: [JumpThreading] In updatePredecessorProfileMetadata, stop searching predecessor when the current bb is an unreachable single bb loop .

LGTM

Fri, Jul 26, 12:17 PM · Restricted Project
efriedma added inline comments to D65234: [CodeGen]: don't treat structures returned in registers as memory inputs.
Fri, Jul 26, 11:48 AM · Restricted Project
efriedma updated subscribers of D65310: [JumpThreading] In updatePredecessorProfileMetadata, stop searching predecessor when the current bb is an unreachable single bb loop .

Please rebase against trunk.

Fri, Jul 26, 11:34 AM · Restricted Project
efriedma added inline comments to D65324: [ARM][ParallelDSP] Convert to function pass.
Fri, Jul 26, 11:25 AM · Restricted Project

Thu, Jul 25

efriedma added a comment to D65310: [JumpThreading] In updatePredecessorProfileMetadata, stop searching predecessor when the current bb is an unreachable single bb loop .

Is there any reason to expect that unreachable loops have exactly one block?

Thu, Jul 25, 5:46 PM · Restricted Project
efriedma added inline comments to D65234: [CodeGen]: don't treat structures returned in registers as memory inputs.
Thu, Jul 25, 1:41 PM · Restricted Project
efriedma accepted D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.

LGTM

Thu, Jul 25, 1:27 PM · Restricted Project
efriedma accepted D64866: [PredicateInfo] Replace pointer comparisons with deterministic compares..

LGTM with one nit.

Thu, Jul 25, 1:16 PM · Restricted Project

Wed, Jul 24

efriedma committed rG82e109279d78: [ARM] Remove dead code from ARMConstantIslands. (authored by efriedma).
[ARM] Remove dead code from ARMConstantIslands.
Wed, Jul 24, 4:37 PM
efriedma committed rL366963: [ARM] Remove dead code from ARMConstantIslands..
[ARM] Remove dead code from ARMConstantIslands.
Wed, Jul 24, 4:36 PM
efriedma added a comment to D60613: Make setInitializer() assert that the entire initializer is usable..

I don't think you actually need a recursion guard; you only recurse on ConstantAggregates, and they can't refer to themselves circularly. But maybe it's a good idea anyway to avoid exponential compile-time in certain edge cases.

Wed, Jul 24, 4:06 PM · Restricted Project
efriedma added inline comments to D63972: [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks.
Wed, Jul 24, 3:58 PM · Restricted Project
efriedma added a comment to D65175: [ARM] Transform compare of masked value to shift on Thumb1..

Any reason why not to use this for thumb2 as well?

Wed, Jul 24, 3:50 PM · Restricted Project
efriedma added a comment to D65204: [GVN] Also invalidate users of instructions replaced due to conditionals..

Oh, okay, that makes more sense.

Wed, Jul 24, 3:23 PM · Restricted Project
efriedma added a comment to D64759: [CodeGen] Don't resolve the stack protector frame accesses until PEI.

Do you think this is blocking for this fix to get in?

Wed, Jul 24, 2:58 PM · Restricted Project
efriedma accepted D65222: [IPSCCP] Add assertion to surface cases where we zap returns with overdefined users..

LGTM

Wed, Jul 24, 2:32 PM · Restricted Project
efriedma added a comment to D62871: [Codegen] (X & (C l>>/<< Y)) ==/!= 0 --> ((X <</l>> Y) & C) ==/!= 0 fold.

LGTM with one minor change.

Wed, Jul 24, 2:30 PM · Restricted Project
efriedma added a comment to D64759: [CodeGen] Don't resolve the stack protector frame accesses until PEI.

I suppose the reg scavenger can also spill it if it really can't find another register, but I'm not sure we can do much about it at that point.

Wed, Jul 24, 1:11 PM · Restricted Project
efriedma added a comment to D65204: [GVN] Also invalidate users of instructions replaced due to conditionals..

I'm not sure this fix is actually solving a real issue, as opposed to merely hiding the issue for the given testcase.

Wed, Jul 24, 12:34 PM · Restricted Project

Tue, Jul 23

efriedma added a comment to D64984: [Verifier] Verify all blockaddress constants left over have at least one user..

I'm not sure this is something we can reasonably enforce without making big changes in other areas of the compiler. For example, we call eraseFromParent() all over the place, and some large fraction of those calls could eliminate the last use of a blockaddress.

Tue, Jul 23, 5:24 PM · Restricted Project
efriedma updated the summary of D65175: [ARM] Transform compare of masked value to shift on Thumb1..
Tue, Jul 23, 5:11 PM · Restricted Project
efriedma created D65175: [ARM] Transform compare of masked value to shift on Thumb1..
Tue, Jul 23, 5:06 PM · Restricted Project
efriedma accepted D64816: [PredicateInfo] Use SmallVector instead of SmallPtrSet..

LGTM

Tue, Jul 23, 2:57 PM · Restricted Project
efriedma committed rGc69273fa1072: [docs] Clarify where the indirect UB due to write-write races comes from (authored by efriedma).
[docs] Clarify where the indirect UB due to write-write races comes from
Tue, Jul 23, 2:54 PM
efriedma committed rL366855: [docs] Clarify where the indirect UB due to write-write races comes from.
[docs] Clarify where the indirect UB due to write-write races comes from
Tue, Jul 23, 2:54 PM
efriedma closed D65134: Clarify where the indirect UB due to write-write races comes from.

r366855.

Tue, Jul 23, 2:54 PM · Restricted Project
efriedma added inline comments to D64866: [PredicateInfo] Replace pointer comparisons with deterministic compares..
Tue, Jul 23, 2:36 PM · Restricted Project