Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (141 w, 6 h)

Recent Activity

Today

efriedma added a comment to D61095: [AArch64][Windows] Compute function length correctly in unwind tables..

Well, I guess there are sort of two potential approaches to emitting it "later". One, we could emit something like ".word (endfunc-beginfunc)/4+0x10800000", as long as we have some conservative estimate we could use to prove that we don't need to split the unwind data. The other would be to use a dedicated, relaxable fragment, which could grow to accommodate splitting the unwind data if necessary. The dedicated fragment is probably a bit more bulletproof, and more consistent with the way we handle relaxation in other contexts, but it would require a lot of refactoring to implement.

Wed, Apr 24, 4:38 PM · Restricted Project
efriedma created D61095: [AArch64][Windows] Compute function length correctly in unwind tables..
Wed, Apr 24, 2:59 PM · Restricted Project
efriedma added inline comments to D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube.
Wed, Apr 24, 11:19 AM · Restricted Project
efriedma added a comment to D60349: [COFF, ARM64] Fix ABI implementation of struct returns.

For NotPod, it is aggregate which is specific in the document

Wed, Apr 24, 9:40 AM · Restricted Project

Yesterday

efriedma added inline comments to D60348: [COFF, ARM64] Fix ABI implementation of struct returns.
Tue, Apr 23, 7:12 PM · Restricted Project
efriedma added a comment to D60349: [COFF, ARM64] Fix ABI implementation of struct returns.

It looks like there's some missing documentation in the ARM64 ABI document involving homogeneous aggregates... in particular, it looks like non-POD types are never homogeneous, or something along those lines. I guess we can address that in a followup, though.

Tue, Apr 23, 4:17 PM · Restricted Project
efriedma accepted D60239: [Lint] Permit aliasing noalias readonly arguments.

LGTM. It seems easy to trigger false positives with this lint, so I'm not sure how useful it is in general, but this is clearly an improvement.

Tue, Apr 23, 1:37 PM · Restricted Project
efriedma added a comment to D60052: Add Connex vector processor back end.

Just quickly reviewing the target-independent changes.

Tue, Apr 23, 1:29 PM · Restricted Project
efriedma updated the diff for D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols..

Partially addresses review comments. Still need to think about the mapping symbol vectors a bit more.

Tue, Apr 23, 12:14 PM · Restricted Project
efriedma added a comment to D60348: [COFF, ARM64] Fix ABI implementation of struct returns.

Is it possible to enable fast-isel and/or global-isel for ARM64 Windows? Do we need additional code for them? (I'm okay with making those changes separate patches, but it should be easy to at least add testcases now.)

Tue, Apr 23, 12:06 PM · Restricted Project
efriedma added inline comments to D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols..
Tue, Apr 23, 12:04 PM · Restricted Project

Mon, Apr 22

efriedma added a comment to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.

that's a case I don't think can be done in C even with GNU C extensions

Mon, Apr 22, 5:09 PM · Restricted Project
efriedma added inline comments to D60349: [COFF, ARM64] Fix ABI implementation of struct returns.
Mon, Apr 22, 4:24 PM · Restricted Project
efriedma added inline comments to D60348: [COFF, ARM64] Fix ABI implementation of struct returns.
Mon, Apr 22, 3:34 PM · Restricted Project
efriedma added a comment to D60348: [COFF, ARM64] Fix ABI implementation of struct returns.

Do you need to modify AArch64TargetLowering::isEligibleForTailCallOptimization to prevent a tail call in cases where the tail call would return the wrong value?

Mon, Apr 22, 2:09 PM · Restricted Project
efriedma added a comment to D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols..

In terms of how to generally improve disassembleObject, I guess there are a few different ways to think about it...

Mon, Apr 22, 12:53 PM · Restricted Project
efriedma added inline comments to D60349: [COFF, ARM64] Fix ABI implementation of struct returns.
Mon, Apr 22, 12:50 PM · Restricted Project
efriedma added a comment to D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube.

so the easiest way to fix this bug is to remove !LegalOperations before ||

Mon, Apr 22, 11:36 AM · Restricted Project

Fri, Apr 19

efriedma created D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols..
Fri, Apr 19, 4:38 PM · Restricted Project
efriedma committed rG1810339bc305: [AArch64] Fix checks for AArch64MCExpr::VK_SABS flag. (authored by efriedma).
[AArch64] Fix checks for AArch64MCExpr::VK_SABS flag.
Fri, Apr 19, 2:57 PM
efriedma committed rL358788: [AArch64] Fix checks for AArch64MCExpr::VK_SABS flag..
[AArch64] Fix checks for AArch64MCExpr::VK_SABS flag.
Fri, Apr 19, 2:56 PM
efriedma closed D60596: Fix checks for AArch64MCExpr::VK_SABS flag.
Fri, Apr 19, 2:56 PM · Restricted Project
efriedma accepted D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.

LGTM

Fri, Apr 19, 2:46 PM · Restricted Project
efriedma added inline comments to D60839: [ScheduleDAGInstrs] Compute topological ordering on demand..
Fri, Apr 19, 2:46 PM · Restricted Project
efriedma accepted D60909: [X86] Fix stack probing on x32 (PR41477).

LGTM

Fri, Apr 19, 1:36 PM · Restricted Project
efriedma added inline comments to D60909: [X86] Fix stack probing on x32 (PR41477).
Fri, Apr 19, 12:08 PM · Restricted Project
efriedma added a comment to D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube.

In former condition, such transformation is acceptable even operation ADDE is illegal and it only cares about type legality because illegal operation will be legalized later in DAG legalization phase

Fri, Apr 19, 11:08 AM · Restricted Project

Thu, Apr 18

efriedma added a comment to D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube.

Oh, I see, we start with an i32 ADDE, then https://github.com/llvm/llvm-project/blob/e7fe6dd5edb828e702d02c7cdc6565ace4c84f5b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L10232 narrows the operation. We probably shouldn't perform that transform if it would produce an illegal ADDE.

Thu, Apr 18, 10:37 PM · Restricted Project
efriedma added a comment to D60890: [AArch64] splat before (f)mul to allow mul-by-element isel.

The constant cases look better, but I'm not sure if this is a win if both operands are variables.

Thu, Apr 18, 4:45 PM · Restricted Project
efriedma added a comment to D60811: [PowerPC] Fix wrong ElemSIze when calling isConsecutiveLS().

I guess that's right, technically; "getScalarSizeInBits() / 8" will round down, and getStoreSize() will round up. But still, please fix the code so it's clear that check is happening.

Thu, Apr 18, 3:34 PM · Restricted Project
efriedma requested changes to D60705: [ARM] Turn some undefined encoding bits into mandatory 1s..

The use of "?" here doesn't seem right. All those instructions where you're adding a "?" do in fact require that the bit in question must be zero.

Thu, Apr 18, 3:19 PM · Restricted Project
efriedma added a comment to D60811: [PowerPC] Fix wrong ElemSIze when calling isConsecutiveLS().

This fix is wrong. For vectors where the element type is not byte-sized, loads can't be "consecutive" in the sense of this function; the store size of <2 x i1> is one byte.

Thu, Apr 18, 2:38 PM · Restricted Project
efriedma accepted D60874: [ARM][FIX] Add missing f16.lane.vldN/vstN lowering.

LGTM

Thu, Apr 18, 11:26 AM · Restricted Project
efriedma added a comment to D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube.

ADDC etc. are deprecated in favor of ADDCARRY. Nothing should generate ADDC etc. unless they're marked "Legal" for the type in question... and they should be Expand by default.

Thu, Apr 18, 9:26 AM · Restricted Project

Wed, Apr 17

efriedma created D60840: [AArch64][MC] Reject "add x0, x1, w2, lsl #1" etc..
Wed, Apr 17, 2:53 PM · Restricted Project
efriedma added inline comments to D60832: [MemorySSA] LCSSA preserves MemorySSA..
Wed, Apr 17, 2:19 PM · Restricted Project
efriedma added inline comments to D60214: [DAGCombiner] move splat-shuffle after binop with splat constant.
Wed, Apr 17, 12:50 PM · Restricted Project

Mon, Apr 15

efriedma added a comment to D60294: [DAGCombiner] [CodeGenPrepare] Split large offsets from base addresses.

I wasn't thinking in terms of keeping the addressing mode legal, just avoiding destroying the work of constant hoisting. Constant hoisting won't split a constant in the way you're suggesting. And it's relatively easy to write patterns to split "load x+c" in the most efficient way if "c" has a single use. I guess the more restrictive version would allow splitting single-use constants as a DAGCombine, or earlier? Not sure why you'd want to, though.

Mon, Apr 15, 4:35 PM · Restricted Project
efriedma added a comment to D60674: [X86] Restore the pavg intrinsics..

Okay, thanks.

Mon, Apr 15, 3:57 PM · Restricted Project, Restricted Project
efriedma added a comment to D60674: [X86] Restore the pavg intrinsics..

Though I modified the avx512 intrinsics to not have masking built in.

Mon, Apr 15, 12:35 PM · Restricted Project, Restricted Project

Fri, Apr 12

efriedma added inline comments to D60349: [COFF, ARM64] Fix ABI implementation of struct returns.
Fri, Apr 12, 12:53 PM · Restricted Project
efriedma added a comment to D60601: [DAGCombiner] Exploiting more about the transformation of TransformFPLoadStorePair function.

I can't find any previous discussion of this particular check. I agree it isn't necessary for correctness.

Fri, Apr 12, 12:47 PM · Restricted Project
efriedma accepted D60596: Fix checks for AArch64MCExpr::VK_SABS flag.

LGTM

Fri, Apr 12, 12:37 PM · Restricted Project

Tue, Apr 9

efriedma accepted D60319: [ARM] [FIX] Add missing f16 vector operations lowering.

LGTM

Tue, Apr 9, 12:43 PM · Restricted Project
efriedma added inline comments to D60457: [CodeGen] Fixed de-optimization of legalize subvector extract.
Tue, Apr 9, 12:37 PM · Restricted Project

Mon, Apr 8

efriedma created D60427: [ARM] Glue register copies to tail calls..
Mon, Apr 8, 4:40 PM · Restricted Project
efriedma added a comment to D60031: Split tailcallelim into tailcallmark and tailcallelim.

(Probably should be using setPreservesCFG().)

Mon, Apr 8, 11:09 AM · Restricted Project

Fri, Apr 5

efriedma added a comment to D60319: [ARM] [FIX] Add missing f16 vector operations lowering.

Do we not have some existing file we've been using for fp16 tests?

Fri, Apr 5, 12:47 PM · Restricted Project
efriedma added a comment to D60314: [DAGCombiner] Add missing flag to addressing mode check.

Probably better to put it inside the branches for ADD/SUB, to make it clear that it's those specific operations that have a base register?

Fri, Apr 5, 12:18 PM · Restricted Project
efriedma added a comment to D60294: [DAGCombiner] [CodeGenPrepare] Split large offsets from base addresses.

Should we care about the number of uses of N0 here? It should generally be "safe" to reassociate in cases where it only has one use.

Fri, Apr 5, 12:02 PM · Restricted Project

Thu, Apr 4

efriedma added a comment to D59050: [InterleavedAccessAnalysis] Use unordered_map to avoid tombstone insertion..

That layout generally makes sense, sure. Maybe you could get away with one bit per slot by sticking a bit to distinguish between either empty/tombstone into the main table, but I guess it's not really a big deal either way if you're talking about one or two bits per entry. I guess two bits makes failed lookups faster.

Thu, Apr 4, 3:03 PM · Restricted Project
efriedma updated subscribers of D59990: AMDGPU. Divergence driven ISel. Assign register class for cross block values according to the divergence..

Adding llvm-commits to the CC. Please be more careful about that in the future... see http://llvm.org/docs/Phabricator.html

Thu, Apr 4, 12:26 PM
efriedma added inline comments to D59990: AMDGPU. Divergence driven ISel. Assign register class for cross block values according to the divergence..
Thu, Apr 4, 12:06 PM
efriedma added a comment to D60266: [LoopUnroll] Rotate loop, when optimizing for size and can fully unroll a loop..

Why do we need to rotate the loop before unrolling? llvm::UnrollLoop currently refuses to unroll loops where the latch is an unconditional branch, but that isn't a fundamental limitation, as far as I can tell. We already support unrolling loops where the latch is not the exit branch; allowing loops where the latch doesn't exit at all is a minor extension. Granted, it might be more efficient to explicitly rotate the loop before unrolling, so we don't clone quite so much code.

Thu, Apr 4, 11:58 AM · Restricted Project
efriedma accepted D60125: [ScheduleDAGRRList] Recompute topological ordering on demand..

LGTM

Thu, Apr 4, 11:58 AM · Restricted Project

Wed, Apr 3

efriedma added a comment to D60224: [TargetLowering] Extend bool args to inline-asm according to getBooleanType.

What's the alternative? We can't dictate the behavior of target-specific constraints from target-independent code; some targets actually have boolean registers.

Wed, Apr 3, 2:13 PM · Restricted Project
efriedma added inline comments to D60232: [WebAssembly] EmscriptenEHSjLj: Don't abort if __THREW__ is defined.
Wed, Apr 3, 2:10 PM · Restricted Project
efriedma added inline comments to D60224: [TargetLowering] Extend bool args to inline-asm according to getBooleanType.
Wed, Apr 3, 1:25 PM · Restricted Project
efriedma added inline comments to D60125: [ScheduleDAGRRList] Recompute topological ordering on demand..
Wed, Apr 3, 1:13 PM · Restricted Project
efriedma accepted D60187: [ScheduleDAG] Add statistics for maintaining the topological order..

LGTM

Wed, Apr 3, 12:38 PM · Restricted Project
efriedma added inline comments to D59990: AMDGPU. Divergence driven ISel. Assign register class for cross block values according to the divergence..
Wed, Apr 3, 11:55 AM

Tue, Apr 2

efriedma added inline comments to D60125: [ScheduleDAGRRList] Recompute topological ordering on demand..
Tue, Apr 2, 12:49 PM · Restricted Project
efriedma added inline comments to D59990: AMDGPU. Divergence driven ISel. Assign register class for cross block values according to the divergence..
Tue, Apr 2, 12:25 PM
efriedma added inline comments to D60125: [ScheduleDAGRRList] Recompute topological ordering on demand..
Tue, Apr 2, 12:04 PM · Restricted Project

Mon, Apr 1

efriedma added a comment to D58560: [AST] Set 'MayAlias' instead of 'MustAlias' in AliasSets for PHI nodes (bugzilla bug#36801).

Spent a bit more time looking, and thinking, and I think I have a clearer picture of what's happening.

Mon, Apr 1, 7:02 PM · Restricted Project
efriedma added a comment to D58560: [AST] Set 'MayAlias' instead of 'MustAlias' in AliasSets for PHI nodes (bugzilla bug#36801).

I'll look a little more at D58746, but that seems more like the right direction

Mon, Apr 1, 6:17 PM · Restricted Project
efriedma committed rG3813fe0bda83: [ARM] Optimize expressions like "return x != 0;" for Thumb1. (authored by efriedma).
[ARM] Optimize expressions like "return x != 0;" for Thumb1.
Mon, Apr 1, 5:01 PM
efriedma committed rL357437: [ARM] Optimize expressions like "return x != 0;" for Thumb1..
[ARM] Optimize expressions like "return x != 0;" for Thumb1.
Mon, Apr 1, 5:00 PM
efriedma closed D59616: [ARM] Optimize expressions like "return x != 0;" for Thumb1..
Mon, Apr 1, 4:59 PM · Restricted Project
efriedma committed rG73af6ef2e752: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz. (authored by efriedma).
[ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz.
Mon, Apr 1, 4:57 PM
efriedma committed rL357436: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz..
[ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz.
Mon, Apr 1, 4:57 PM
efriedma closed D59385: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz..
Mon, Apr 1, 4:57 PM · Restricted Project
efriedma added a comment to D60033: [MSP430] Expand Atomic nodes.

There are no multiple threads, etc. on msp430

Mon, Apr 1, 4:30 PM · Restricted Project
efriedma added inline comments to D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.
Mon, Apr 1, 3:53 PM · Restricted Project
efriedma added a comment to D60090: [ARM] Update check for CBZ in Ifcvt.

Yes, please refactor the code. We define a bunch of helpers in ARMBaseInstrInfo.h; that's probably fine.

Mon, Apr 1, 3:41 PM · Restricted Project
efriedma added a comment to D60021: InstSimplify: Fold round intrinsics from sitofp.

I'm not sure where or if there are official docs for libm

Mon, Apr 1, 11:56 AM
efriedma added inline comments to D60031: Split tailcallelim into tailcallmark and tailcallelim.
Mon, Apr 1, 11:20 AM · Restricted Project

Thu, Mar 28

efriedma added inline comments to D57493: [RISCV] Put data smaller than eight bytes to small data section.
Thu, Mar 28, 4:17 PM · Restricted Project
efriedma updated the diff for D59385: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz..

Update test to use -verify-machineinstrs, and to use CHECK-LABEL in a way that's less likely to cause confusion.

Thu, Mar 28, 2:47 PM · Restricted Project
efriedma updated the diff for D59616: [ARM] Optimize expressions like "return x != 0;" for Thumb1..

Added a couple more tests, including one that generates the subs+sbcs+lsls sequence.

Thu, Mar 28, 2:34 PM · Restricted Project
efriedma committed rG3dd72ea810db: [MC] Fix floating-point literal lexing. (authored by efriedma).
[MC] Fix floating-point literal lexing.
Thu, Mar 28, 2:13 PM
efriedma committed rL357214: [MC] Fix floating-point literal lexing..
[MC] Fix floating-point literal lexing.
Thu, Mar 28, 2:13 PM
efriedma closed D57321: Fix LexFloatLiteral Lexing.
Thu, Mar 28, 2:13 PM · Restricted Project
efriedma committed rG96f295e23bed: [InterleavedAccessPass] Don't increase the number of bytes loaded. (authored by efriedma).
[InterleavedAccessPass] Don't increase the number of bytes loaded.
Thu, Mar 28, 1:45 PM
efriedma committed rL357212: [InterleavedAccessPass] Don't increase the number of bytes loaded..
[InterleavedAccessPass] Don't increase the number of bytes loaded.
Thu, Mar 28, 1:45 PM
efriedma closed D59954: [InterleavedAccessPass] Don't increase the number of bytes loaded..
Thu, Mar 28, 1:45 PM · Restricted Project
efriedma accepted D59039: [DAGCombine] Allow GatherAllAliases to pass through inline asm calls and glued nodes..

LGTM

Thu, Mar 28, 1:05 PM · Restricted Project
efriedma accepted D59218: [LSR] Fix signed overflow in GenerateCrossUseConstantOffsets..

LGTM

Thu, Mar 28, 12:16 PM · Restricted Project
efriedma created D59954: [InterleavedAccessPass] Don't increase the number of bytes loaded..
Thu, Mar 28, 12:10 PM · Restricted Project
efriedma requested changes to D59080: Merge of global constants does not happen when constants have common linkage.

This isn't correct; GlobalMerge is not legal for symbols with weak linkage, and there isn't any way to correctly emit an alias with "common" linkage in ELF.

Thu, Mar 28, 11:19 AM

Wed, Mar 27

efriedma accepted D59868: [ARM] Remove dead function ARMMCCodeEmitter::getSOImmOpValue.

LGTM

Wed, Mar 27, 2:52 PM · Restricted Project
efriedma accepted D57321: Fix LexFloatLiteral Lexing.

LGTM. Thanks for sticking with this for so many rounds of review.

Wed, Mar 27, 2:01 PM · Restricted Project
efriedma committed rGc388bfa23040: [ARM] Don't confuse the scheduler for very large VLDMDIA etc. (authored by efriedma).
[ARM] Don't confuse the scheduler for very large VLDMDIA etc.
Wed, Mar 27, 11:33 AM
efriedma committed rL357109: [ARM] Don't confuse the scheduler for very large VLDMDIA etc..
[ARM] Don't confuse the scheduler for very large VLDMDIA etc.
Wed, Mar 27, 11:32 AM
efriedma closed D59834: [ARM] Don't confuse the scheduler for very large VLDMDIA etc..
Wed, Mar 27, 11:32 AM · Restricted Project

Tue, Mar 26

efriedma added a comment to D59385: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz..

When I tried your example it was pushing r7 and popping r3. I presume this is OK, providing r7 isn't modified?

Tue, Mar 26, 12:14 PM · Restricted Project
efriedma added a comment to D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics.

For that particular case, I think we could suppress the false positive using DiagRuntimeBehavior. How many total false positives are we talking about, and how many can the compiler statically prove are unreachable?

Tue, Mar 26, 12:08 PM · Restricted Project
efriedma added a comment to D59713: [ARM] Add missing memory operands to a bunch of instructions..

https://reviews.llvm.org/D59834

Tue, Mar 26, 12:00 PM · Restricted Project
efriedma created D59834: [ARM] Don't confuse the scheduler for very large VLDMDIA etc..
Tue, Mar 26, 12:00 PM · Restricted Project

Mar 25 2019

efriedma committed rG1e5d569c8c78: [ARM] Add missing memory operands to a bunch of instructions. (authored by efriedma).
[ARM] Add missing memory operands to a bunch of instructions.
Mar 25 2019, 3:42 PM