Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

efriedma added a comment to D87340: [EarlyCSE] Handle masked loads and stores.

Overall this seems like a natural extension of the existing EarlyCSE handling of load/store. I don't see any major issues.

Thu, Sep 17, 9:56 PM · Restricted Project
efriedma added a comment to D87691: [EarlyCSE] Small refactoring changes, NFC.

Mostly looks fine; couple minor suggestions.

Thu, Sep 17, 9:22 PM · Restricted Project
efriedma accepted D82269: [TRE][NFC] Refactor Basic Block Processing.

LGTM. Sorry about the delay.

Thu, Sep 17, 6:02 PM · Restricted Project
efriedma added a comment to D85614: [TRE] Reland: allow TRE for non-capturing calls..

Among all the attributes that can be attached to an argument of a tail call, byval is unusual. It significantly changes the semantics of the call: when the call executes, instead of passing the pointer itself, it makes a copy of the data behind the pointer.

Thu, Sep 17, 5:59 PM · Restricted Project
efriedma added inline comments to D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR..
Thu, Sep 17, 4:24 PM · Restricted Project
efriedma added a comment to D87817: [LICM][Coroutine] Don't sink stores to coroutine suspend point..

If there are transformations that are specifically illegal with corouties, I'd like to see a section in https://llvm.org/docs/Coroutines.html describing what transforms are and are not legal. Just saying that the current version of the coroutine lowering code can't handle it isn't sufficient.

Thu, Sep 17, 4:01 PM · Restricted Project
efriedma accepted D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics.

LGTM

Thu, Sep 17, 3:56 PM · Restricted Project
efriedma accepted D87286: AArch64: make sure jump table entries can reach entire image.

Further, because of MachO limitations on relocation addends (have I said how much I hate that format in the last week? Time to do it again), it can't be anything inside the function, leaving only the function symbol itself.

LLVM seems to accept "adrp x8, (f+1)@PAGE"; does that not do the right thing?

Thu, Sep 17, 3:52 PM · Restricted Project
efriedma added a comment to D87286: AArch64: make sure jump table entries can reach entire image.

Further, because of MachO limitations on relocation addends (have I said how much I hate that format in the last week? Time to do it again), it can't be anything inside the function, leaving only the function symbol itself.

Thu, Sep 17, 3:46 PM · Restricted Project
efriedma added inline comments to D86074: [ARM][MVE] Tail-predication: check get.active.lane.mask's TC value.
Thu, Sep 17, 2:26 PM · Restricted Project
efriedma added a comment to D87827: [SCEVExpander] Support expanding nonintegral pointers with constant base..

I have a proposal in https://bugs.llvm.org/show_bug.cgi?id=46786#c20 which would solve this, among other issues. Not sure when I'll get around to actually implementing it.

Thu, Sep 17, 2:09 PM · Restricted Project
efriedma accepted D87851: [AArch64] Enable implicit null check transformation.

Please fix clang-format warnings. Otherwise LGTM.

Thu, Sep 17, 1:54 PM · Restricted Project
efriedma added a comment to D87842: [SVE] Use NEON for extract_vector_elt when the index is in range..

Please make sure we have test coverage for the SVE-only extracts (e.g. extracting the third element of a <vscale x 2 x i64>).

Thu, Sep 17, 1:39 PM · Restricted Project
efriedma accepted D87843: [SVE] Change definition of reduction ISD nodes to have an SVE vector result type..

LGTM

Thu, Sep 17, 12:23 PM · Restricted Project
efriedma added a comment to D87835: [APFloat] prevent NaN morphing into Inf on conversion (PR43907).

even though it _doesn't_ match what the HW does, because the HW can signal and APFloat can't

Thu, Sep 17, 11:59 AM · Restricted Project
efriedma accepted D85364: [SVE][WIP] Implement lowering for fixed width select.

The other codepaths should do the right thing in theory, so I'm fine with leaving them, I think.

Thu, Sep 17, 11:37 AM · Restricted Project
efriedma accepted D87771: [AArch64] Emit zext move when the source of the zext is AssertZext or AssertSext.

I'm okay with taking this as an incremental improvement, and then looking into a different approach later.

Thu, Sep 17, 11:29 AM · Restricted Project
efriedma added inline comments to D87842: [SVE] Use NEON for extract_vector_elt when the index is in range..
Thu, Sep 17, 11:23 AM · Restricted Project
efriedma added a comment to D87844: [CodeGen] Fixing inconsistent ABI mangling of vlaues in SelectionDAGBuilder.

If I'm following this correctly, with this change, getABIRegCopyCC has one remaining caller, and that call always returns None. So getABIRegCopyCC is useless, and the CallConv member of RegsForValue is always None? It seems weird that all this code got built up around a useless operation, but I guess that sort of thing can happen.

Thu, Sep 17, 11:08 AM · Restricted Project

Wed, Sep 16

efriedma added inline comments to D86928: [SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-store.ll.
Wed, Sep 16, 7:33 PM · Restricted Project
efriedma accepted D86510: [compiler-rt] Fix atomic support functions on 32-bit architectures.

LGTM

Wed, Sep 16, 7:32 PM · Restricted Project
efriedma accepted D86816: [LoopDelete][Assume] Allow deleting loops with assumes.

Sure, seems fine for now.

Wed, Sep 16, 7:22 PM · Restricted Project
efriedma accepted D87408: [NFC] EliminateDuplicatePHINodes(): small-size optimization: if there are <= 32 PHI's, O(n^2) algo is faster (geomean -0.08%).

LGTM

Wed, Sep 16, 7:19 PM · Restricted Project
efriedma added inline comments to D85364: [SVE][WIP] Implement lowering for fixed width select.
Wed, Sep 16, 7:12 PM · Restricted Project
efriedma accepted D87790: [ARM] Select f32 constants with vmov.f16.

On a side-note, should we be trying to emit movw+vmov instead of a constant-pool load under any circumstances?

Wed, Sep 16, 7:02 PM · Restricted Project
efriedma accepted D87228: [Lint] Add check for intrinsic get.active.lane.mask.

LGTM

Wed, Sep 16, 6:36 PM · Restricted Project
efriedma added inline comments to D86074: [ARM][MVE] Tail-predication: check get.active.lane.mask's TC value.
Wed, Sep 16, 6:34 PM · Restricted Project
efriedma accepted D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors.

LGTM

Wed, Sep 16, 4:30 PM · Restricted Project
efriedma accepted D86478: [ARM][CMSE] Issue an error if passing arguments through memory across security boundary.

LGTM

Wed, Sep 16, 4:29 PM · Restricted Project
efriedma added a comment to D87708: [DAG] DAGTypeLegalizer::GenWidenVectorTruncStores - ensure correct extraction index (PR42046).

Err, actually, thinking about it a bit more, we should probably delete GenWidenVectorTruncStores in favor of using scalarizeVectorStore(), since they do basically the same thing.

Wed, Sep 16, 4:25 PM · Restricted Project
efriedma accepted D87708: [DAG] DAGTypeLegalizer::GenWidenVectorTruncStores - ensure correct extraction index (PR42046).

This is by inspection - as was mentioned on the ticket, this code isn't active at all in the tests and I've not been able to get it to fire

Wed, Sep 16, 4:21 PM · Restricted Project
efriedma accepted D87232: [SVE][CodeGen] Lower floating point -> integer conversions.
Wed, Sep 16, 4:05 PM · Restricted Project
efriedma accepted D87789: [ARM] Constant fold VMOVrh.

LGTM

Wed, Sep 16, 4:01 PM · Restricted Project
efriedma added a comment to D87796: [SVE][WIP] Lower fixed length VECREDUCE_ADD to Scalable.

I think the NEON code is trying to avoid producing an extra instruction if the result ends up getting used by a vector operation. SelectionDAG technically allows instructions that produce an i64 result to return it in a floating-point register, but it doesn't work very well in practice: the operations that consume it will force it back into a GPR. This is one of the issues GlobalISel's RegBankSelect solves.

Wed, Sep 16, 2:41 PM · Restricted Project
efriedma updated subscribers of D87771: [AArch64] Emit zext move when the source of the zext is AssertZext or AssertSext.
Wed, Sep 16, 12:31 PM · Restricted Project
efriedma accepted D87615: [X86] Fix stack alignment on 32-bit Solaris/x86.

LGTM

Wed, Sep 16, 11:18 AM · Restricted Project, Restricted Project, Restricted Project

Tue, Sep 15

efriedma added inline comments to D87651: [AArch64][SVE] Implement extractelement of i1 vectors..
Tue, Sep 15, 2:19 PM · Restricted Project
efriedma added a comment to D87615: [X86] Fix stack alignment on 32-bit Solaris/x86.

Also, it would be nice to have some regression test coverage; add a Solaris RUN line to llvm/test/CodeGen/X86/stack-align2.ll ?

Tue, Sep 15, 12:30 PM · Restricted Project, Restricted Project, Restricted Project
efriedma added inline comments to D87615: [X86] Fix stack alignment on 32-bit Solaris/x86.
Tue, Sep 15, 12:23 PM · Restricted Project, Restricted Project, Restricted Project
efriedma added inline comments to D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors.
Tue, Sep 15, 12:02 PM · Restricted Project

Mon, Sep 14

efriedma requested review of D87651: [AArch64][SVE] Implement extractelement of i1 vectors..
Mon, Sep 14, 4:18 PM · Restricted Project
efriedma accepted D87370: [llvm-readobj] [ARMWinEH] Print ARM64 packed unwind info.

LGTM

Mon, Sep 14, 2:05 PM · Restricted Project
efriedma added inline comments to D87276: [ARM] Recognize "double extend" reduction patterns.
Mon, Sep 14, 2:04 PM · Restricted Project
efriedma added inline comments to D86074: [ARM][MVE] Tail-predication: check get.active.lane.mask's TC value.
Mon, Sep 14, 1:59 PM · Restricted Project
efriedma added inline comments to D86928: [SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-store.ll.
Mon, Sep 14, 12:06 PM · Restricted Project
efriedma added inline comments to D87430: [ARM] Add heuristic to avoid lowering calls to blx for Thumb1 in ARMTargetLowering::LowerCall.
Mon, Sep 14, 12:02 PM · Restricted Project
efriedma added inline comments to D85364: [SVE][WIP] Implement lowering for fixed width select.
Mon, Sep 14, 11:37 AM · Restricted Project
efriedma added inline comments to D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors.
Mon, Sep 14, 11:30 AM · Restricted Project
efriedma added inline comments to D86078: [AArch64] Improved lowering for saturating float to int..
Mon, Sep 14, 11:22 AM · Restricted Project
efriedma added a comment to D87615: [X86] Fix stack alignment on 32-bit Solaris/x86.
In D87615#2271297, @ro wrote:

Would it be possible to add some tests?

Probably not reliably: the tests that usually fail bye the hundreds happen to PASS once in a while when the stack pointer is 16-byte aligned
by accident. Otherwise, the failure is prominent enough, I'd think.

Mon, Sep 14, 11:09 AM · Restricted Project, Restricted Project, Restricted Project

Fri, Sep 11

efriedma committed rGd751f86189a7: [ConstantFold] Make areGlobalsPotentiallyEqual less aggressive. (authored by efriedma).
[ConstantFold] Make areGlobalsPotentiallyEqual less aggressive.
Fri, Sep 11, 5:23 PM
efriedma closed D87123: [ConstantFold] Make areGlobalsPotentiallyEqual less aggressive..
Fri, Sep 11, 5:23 PM · Restricted Project
efriedma added a comment to D87408: [NFC] EliminateDuplicatePHINodes(): small-size optimization: if there are <= 32 PHI's, O(n^2) algo is faster (geomean -0.08%).

The point of the cutoff wouldn't be to handle anything in the LLVM testsuite; it would be to prevent it from blowing up on someone's giant machine-generated function which isn't represented in the testuite.

Fri, Sep 11, 4:47 PM · Restricted Project
efriedma committed rG37f2776d1af2: [ConstantFold] Fold binary arithmetic on scalable vector splats. (authored by efriedma).
[ConstantFold] Fold binary arithmetic on scalable vector splats.
Fri, Sep 11, 4:42 PM
efriedma closed D87422: [ConstantFold] Fold binary arithmetic on scalable vector splats..
Fri, Sep 11, 4:42 PM · Restricted Project
efriedma added inline comments to D86928: [SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-store.ll.
Fri, Sep 11, 4:39 PM · Restricted Project
efriedma added inline comments to D87232: [SVE][CodeGen] Lower floating point -> integer conversions.
Fri, Sep 11, 4:18 PM · Restricted Project
efriedma accepted D82676: [CGP] Prevent optimizePhiType from iterating forever.

LGTM

Fri, Sep 11, 4:16 PM · Restricted Project
efriedma added a comment to D87490: [MachineInstr] return mayAlias for not mayLoadOrStore instructions..

Maybe we should be explicitly checking for isCall here? It's not really right to assume instructions that any instruction that doesn't have mayLoad/mayStore modifies memory.

Fri, Sep 11, 4:10 PM · Restricted Project
efriedma accepted D87439: [SVE] In LoopIdiomRecognize::isLegalStore bail out for scalable vectors.

LGTM

Fri, Sep 11, 3:59 PM · Restricted Project
efriedma accepted D86949: [ARM] Fix so immediates and pc relative checks.

LGTM

Fri, Sep 11, 3:52 PM · Restricted Project
efriedma accepted D87209: [SelectionDAG][X86][ARM][AArch64] Add ISD opcode for __builtin_parity. Expand it to shifts and xors..

LGTM

Fri, Sep 11, 3:46 PM · Restricted Project
efriedma added inline comments to D87370: [llvm-readobj] [ARMWinEH] Print ARM64 packed unwind info.
Fri, Sep 11, 3:45 PM · Restricted Project
efriedma added a comment to D86066: IR: Merge MemCpyInlineInst and MemCpyInst.

Having a bunch of boolean modifiers floating around on memcpy sounds miserable for code to deal with; it makes it hard to code to just ignore the "complicated" memcpy operations if it doesn't want to reason about them. (It's already messy just dealing with volatile operations.)

Fri, Sep 11, 3:40 PM · Restricted Project
efriedma added inline comments to D86078: [AArch64] Improved lowering for saturating float to int..
Fri, Sep 11, 3:28 PM · Restricted Project
efriedma added a comment to D87448: [CodeGen] [WinException] Only produce handler data at the end of the function if needed.

I think it's called this out in the comments on the aarch64 implementation, but ultimately, I think we want specialized MCFragments for pdata/xdata. The current AArch64 implementation currently has a couple FIXMEs which can't be fixed in the current architecture (in particular, it doesn't interact with relaxation correctly).

Fri, Sep 11, 3:21 PM · Restricted Project
efriedma added a comment to D86233: [LangRef] Define mustprogress attribute.

Maybe worth noting somewhere that willreturn implies mustprogress.

Fri, Sep 11, 2:55 PM · Restricted Project
efriedma added a comment to D87286: AArch64: make sure jump table entries can reach entire image.

Instead of trying to make the table relative to the jump/adr, can we use the approach we use for compressed jump tables (using the address of the first destination as the base) unconditionally? Seems less complicated overall. I guess making it work in general costs one extra instruction in general: if you want to handle functions larger than the range of adr, you need to use adrp instead of adr. But functions that large are rare, and the potential upside from duplicating jumptable jumps is pretty large.

Fri, Sep 11, 2:41 PM · Restricted Project
efriedma added inline comments to D87430: [ARM] Add heuristic to avoid lowering calls to blx for Thumb1 in ARMTargetLowering::LowerCall.
Fri, Sep 11, 2:35 PM · Restricted Project
efriedma added inline comments to D87209: [SelectionDAG][X86][ARM][AArch64] Add ISD opcode for __builtin_parity. Expand it to shifts and xors..
Fri, Sep 11, 2:22 PM · Restricted Project

Thu, Sep 10

efriedma added inline comments to D87439: [SVE] In LoopIdiomRecognize::isLegalStore bail out for scalable vectors.
Thu, Sep 10, 7:00 PM · Restricted Project
efriedma accepted D87392: [JumpThreading] Fix an incorrect Modified status.

LGTM

Thu, Sep 10, 6:55 PM · Restricted Project
efriedma updated the diff for D87422: [ConstantFold] Fold binary arithmetic on scalable vector splats..

Add an extra test so the divide path is exercised.

Thu, Sep 10, 6:51 PM · Restricted Project
efriedma accepted D87447: [CodeGen] [WinException] Remove a redundant explicit section switch for aarch64.

LGTM

Thu, Sep 10, 6:42 PM · Restricted Project
efriedma added a comment to D87371: [MC] [Win64EH] Try to generate packed unwind info where possible.

the length of the synthesized canonical prologue might differ a little from the actual one maybe

Thu, Sep 10, 6:33 PM · Restricted Project
efriedma accepted D87369: [MC] [Win64EH] Write packed ARM64 epilogs if possible.

LGTM

Thu, Sep 10, 6:12 PM · Restricted Project
efriedma accepted D87367: [MC] [Win64EH] Canonicalize ARM64 unwind opcodes.

LGTM

Thu, Sep 10, 6:10 PM · Restricted Project
efriedma added inline comments to D87370: [llvm-readobj] [ARMWinEH] Print ARM64 packed unwind info.
Thu, Sep 10, 6:05 PM · Restricted Project
efriedma accepted D87286: AArch64: make sure jump table entries can reach entire image.

LGTM, but I still would like to see some numbers before merge.

Thu, Sep 10, 4:52 PM · Restricted Project
efriedma added inline comments to D87471: [Support] Make building with snmalloc work.
Thu, Sep 10, 4:39 PM · Restricted Project
efriedma accepted D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris.

LGTM

Thu, Sep 10, 4:33 PM · Restricted Project, Restricted Project
efriedma added a comment to D87286: AArch64: make sure jump table entries can reach entire image.

Given that most jump tables are probably compressed anyway, I don't expect any big performance impact. But it would be nice to have some numbers.

Thu, Sep 10, 2:42 PM · Restricted Project
efriedma added inline comments to D87430: [ARM] Add heuristic to avoid lowering calls to blx for Thumb1 in ARMTargetLowering::LowerCall.
Thu, Sep 10, 2:15 PM · Restricted Project
efriedma added inline comments to D87232: [SVE][CodeGen] Lower floating point -> integer conversions.
Thu, Sep 10, 1:39 PM · Restricted Project
efriedma added inline comments to D87232: [SVE][CodeGen] Lower floating point -> integer conversions.
Thu, Sep 10, 12:17 PM · Restricted Project
efriedma added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

I will point out that of all the modern compilers I could test, only Clang trunk deletes volatile stores, even followed by clearly unreachable code: https://compiler-explorer.com/z/EGTofn

Thu, Sep 10, 12:08 PM · Restricted Project
efriedma accepted D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits.

LGTM

Thu, Sep 10, 11:29 AM · Restricted Project
efriedma added inline comments to D86078: [AArch64] Improved lowering for saturating float to int..
Thu, Sep 10, 11:26 AM · Restricted Project
efriedma accepted D87424: [SVE] Bail from VectorUtils heuristics for scalable vectors.

LGTM

Thu, Sep 10, 11:03 AM · Restricted Project
efriedma added a comment to D87471: [Support] Make building with snmalloc work.

You can't blindly specify "-mcx16" without checking the target.

Thu, Sep 10, 10:59 AM · Restricted Project

Wed, Sep 9

efriedma added inline comments to D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits.
Wed, Sep 9, 6:02 PM · Restricted Project
efriedma added inline comments to D87424: [SVE] Bail from VectorUtils heuristics for scalable vectors.
Wed, Sep 9, 5:47 PM · Restricted Project
efriedma added inline comments to D87424: [SVE] Bail from VectorUtils heuristics for scalable vectors.
Wed, Sep 9, 5:46 PM · Restricted Project
efriedma accepted D84940: [JumpThreading] Conditionally freeze its condition when unfolding select.

I'm still a little skeptical about the short-term plan with LTO, but I'm not strongly against it.

Wed, Sep 9, 5:36 PM · Restricted Project
efriedma added inline comments to D86078: [AArch64] Improved lowering for saturating float to int..
Wed, Sep 9, 4:48 PM · Restricted Project
efriedma added inline comments to D86078: [AArch64] Improved lowering for saturating float to int..
Wed, Sep 9, 4:33 PM · Restricted Project
efriedma requested review of D87422: [ConstantFold] Fold binary arithmetic on scalable vector splats..
Wed, Sep 9, 3:39 PM · Restricted Project
efriedma added a comment to D87371: [MC] [Win64EH] Try to generate packed unwind info where possible.

Unfortunately, in practice, this isn't ever matched by the code generated by LLVM

Wed, Sep 9, 2:25 PM · Restricted Project
efriedma added inline comments to D87370: [llvm-readobj] [ARMWinEH] Print ARM64 packed unwind info.
Wed, Sep 9, 2:17 PM · Restricted Project
efriedma added inline comments to D87369: [MC] [Win64EH] Write packed ARM64 epilogs if possible.
Wed, Sep 9, 1:55 PM · Restricted Project