efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (70 w, 2 h)

Recent Activity

Today

efriedma added inline comments to D41149: [CodeGen] Specialize mixed-sign mul-with-overflow (fix PR34920).
Wed, Dec 13, 12:34 PM
efriedma added a comment to D41184: [BuildLibCalls] Cast length argument to the correct integer type.

Instead of scattering casts all over the place, can we just fix TargetLibraryInfoImpl::isValidProtoForLibFunc to reject libcalls which use integers with the wrong width?

Wed, Dec 13, 12:03 PM
efriedma edited reviewers for D40723: [MemCpyOpt] Perform call slot optimizations through GEPs, added: sanjoy, hfinkel, majnemer; removed: rsmith.
Wed, Dec 13, 11:56 AM
efriedma added reviewers for D39963: [RISCV] Add initial RISC-V target and driver support: mgorny, efriedma, bkramer, sylvestre.ledru.

Adding some reviewers to hopefully find someone comfortable reviewing the Linux multilib bits. (Not sure I'm adding the right people; this stuff hasn't been touched for a while, as far as I can tell.)

Wed, Dec 13, 11:39 AM

Yesterday

efriedma updated subscribers of D41104: Set the NoRecurse attribute for the dbg intrinsics..

However, I do wonder, is the attribute propagation semantically correct?

Tue, Dec 12, 4:50 PM
efriedma committed rC320533: [Coverage] Always emit unused coverage mappings in the same order..
[Coverage] Always emit unused coverage mappings in the same order.
Tue, Dec 12, 4:15 PM
efriedma committed rL320533: [Coverage] Always emit unused coverage mappings in the same order..
[Coverage] Always emit unused coverage mappings in the same order.
Tue, Dec 12, 4:15 PM
efriedma closed D41140: [Coverage] Always emit unused coverage mappings in the same order..
Tue, Dec 12, 4:14 PM · Restricted Project
efriedma created D41140: [Coverage] Always emit unused coverage mappings in the same order..
Tue, Dec 12, 4:00 PM · Restricted Project
efriedma added inline comments to D35635: [ARM] Optimize {s,u}{add,sub}.with.overflow.
Tue, Dec 12, 3:52 PM
efriedma added a comment to D41131: [AArch64] Implement stack probing for windows.

How does this interact with https://reviews.llvm.org/D40863 ?

Tue, Dec 12, 3:27 PM
efriedma added a comment to D41131: [AArch64] Implement stack probing for windows.

No, that ends up doing the wrong thing.

Tue, Dec 12, 2:20 PM
efriedma accepted D38942: [DAG] Promote ADDCARRY / SUBCARRY.

LGTM

Tue, Dec 12, 2:12 PM
efriedma added a comment to D41131: [AArch64] Implement stack probing for windows.

How to work around this?? The intended instruction is sub sp, sp, x15, lsl #4

Tue, Dec 12, 1:53 PM
efriedma added a comment to D35267: Pass Divergence Analysis data to selection DAG to drive divergence dependent instruction selection.

In this case w/o bit propagation we're still correct but sub-optimal.

Tue, Dec 12, 12:55 PM
efriedma added a comment to D41122: Tests for D34515.

I guess I didn't explain my comment completely in the other review. The point is that you submit the new tests with CHECK lines that pass on master, then modify CHECK lines in the second patch, to make it clear how the patch modifies the generated code.

Tue, Dec 12, 12:34 PM
efriedma added a comment to D40699: Split IndirectBr critical edges before PGO gen/use passes..

You still never answered the question of what happens when there's multiple indirectbrs whose edges can't be split.

Tue, Dec 12, 11:45 AM
efriedma added a comment to D41096: [X86] Initial support for prefer-vector-width function attribute.

Minor typo: I meant "one function attribute set by clang", not "one clang attribute".

Tue, Dec 12, 11:43 AM
efriedma added a comment to D41096: [X86] Initial support for prefer-vector-width function attribute.

If it finds any vector code, the pass would override the prefer-vector-width attribute to the length of the widest vector in that code.

Tue, Dec 12, 11:41 AM

Mon, Dec 11

efriedma added a comment to D41096: [X86] Initial support for prefer-vector-width function attribute.

After this patch, I plan to start using this subtarget feature in X86ISelLowering.cpp to tell the type legalizer and assorted lowering code not to use 512-bit vectors.

Mon, Dec 11, 4:45 PM
efriedma added a comment to D41029: [JumpTables][PowerPC] Let targets decide which switch instructions are suitable for jump tables.

Generally looks fine, but probably needs a pass from a PPC reviewer.

Mon, Dec 11, 2:23 PM
efriedma added a comment to D38942: [DAG] Promote ADDCARRY / SUBCARRY.

Oh, I see. Yes, sign-extending should work: https://rise4fun.com/Alive/Ux9 . Please add a your explanation as a comment (and fix the code so it actually sign-extends).

Mon, Dec 11, 11:54 AM

Fri, Dec 8

efriedma added a comment to D41044: Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code.

This might be something we want to turn on automatically for -Og.

Fri, Dec 8, 6:04 PM · debug-info
efriedma added inline comments to D41043: Support for an intrinsic "fake.use" (and corresponding operand) representing a use of an operand to aid debugging.
Fri, Dec 8, 6:02 PM
efriedma accepted D35192: [ARM] Use ADDCARRY / SUBCARRY.

LGTM

Fri, Dec 8, 2:37 PM
efriedma added inline comments to D41029: [JumpTables][PowerPC] Let targets decide which switch instructions are suitable for jump tables.
Fri, Dec 8, 2:27 PM
efriedma added a comment to D35267: Pass Divergence Analysis data to selection DAG to drive divergence dependent instruction selection.

Any DAG transformation that change divergent pattern to not-divergent or vice versa is illegal.

Fri, Dec 8, 1:24 PM
efriedma accepted D39946: Relax unaligned access assertion when type is byte aligned.

LGTM

Fri, Dec 8, 1:05 PM
efriedma added inline comments to D41029: [JumpTables][PowerPC] Let targets decide which switch instructions are suitable for jump tables.
Fri, Dec 8, 12:55 PM
efriedma added inline comments to D40940: [ubsan] Use pass_object_size info in bounds checks.
Fri, Dec 8, 12:19 PM
efriedma added a comment to D40940: [ubsan] Use pass_object_size info in bounds checks.

It's interesting to me that these array-bound checks don't seem to use @llvm.objectsize in some form already.

Fri, Dec 8, 11:32 AM
efriedma added a comment to D38942: [DAG] Promote ADDCARRY / SUBCARRY.

I can't see how that DAGCombine transform is actually profitable if the type isn't legal... but I guess we might as well just fix the legalizer so we don't trip over it again.

Fri, Dec 8, 11:11 AM

Wed, Dec 6

efriedma added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

I can reproduce the crash with the following:

Wed, Dec 6, 5:22 PM
efriedma accepted D40209: [DAGCombiner] eliminate shuffle of insert element.

LGTM

Wed, Dec 6, 3:45 PM
efriedma added inline comments to D40863: [AArch64][Darwin] Implement stack probing for static and dynamic stack objects.
Wed, Dec 6, 1:31 PM
efriedma accepted D40612: [InstCombine] canonicalize constant-minus-boolean to select-of-constants.

LGTM. We clearly should be canonicalizing expressions like this, and DAGCombine can reverse the transform if necessary.

Wed, Dec 6, 11:57 AM
efriedma accepted D40412: [InlineFunction] Only replace call if there are VarArgs to forward..

Okay, then this LGTM.

Wed, Dec 6, 11:24 AM
efriedma added a comment to D35267: Pass Divergence Analysis data to selection DAG to drive divergence dependent instruction selection.

Does ReplaceAllUsesWith need to propagate changes to the "IsDivergent" bit?

Wed, Dec 6, 11:18 AM
efriedma added a comment to D35192: [ARM] Use ADDCARRY / SUBCARRY.

Yes, that looks correct.

Wed, Dec 6, 10:53 AM
efriedma added a comment to D40412: [InlineFunction] Only replace call if there are VarArgs to forward..

This change isn't a no-op: the code inside the if statement drops the calling convention/attributes/metadata/etc. on the call. Is that intentional?

Wed, Dec 6, 10:39 AM

Tue, Dec 5

efriedma added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

As testcases with hundred of arguments are unwieldy

Tue, Dec 5, 6:39 PM
efriedma updated subscribers of D40863: [AArch64][Darwin] Implement stack probing for static and dynamic stack objects.
Tue, Dec 5, 3:05 PM

Mon, Dec 4

efriedma added a comment to D40699: Split IndirectBr critical edges before PGO gen/use passes..

It's an improvement in that it can handle single indirectbr predecessor cases.

Mon, Dec 4, 3:44 PM
efriedma added a comment to D28820: Warn when calling a non interrupt function from an interrupt on ARM.

What is the best way to modify the code for this compiler change ?

Mon, Dec 4, 3:35 PM
efriedma added a comment to D40566: Check if MemberExpr arguments are type-dependent..

The part which seems to be throwing you off is "or the class member access expression refers to a member of an unknown specialization". A "member of an unknown specialization" is defined in [temp.dep.type] in the standard.

Mon, Dec 4, 1:08 PM
efriedma added a comment to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.

Looking at LegalizeDAG, when we expand a TruncStore we do it as a store of a truncate which IIUC is strictly bitvector-level interpretation.

Mon, Dec 4, 12:49 PM
efriedma added inline comments to D40792: DAG: Match truncated rotation (PR35487).
Mon, Dec 4, 12:40 PM

Fri, Dec 1

efriedma accepted D40706: Fix function pointer tail calls in armv8-M.base.

Looked a bit more; we're running out of registers because we don't consider r12 an allocatable register in Thumb1 mode. Changing that would be a project way outside the scope of this bugfix, so this is fine as-is for now. LGTM.

Fri, Dec 1, 2:15 PM
efriedma accepted D39758: CodeGen: Fix pointer info in SplitVecOp_EXTRACT_VECTOR_ELT.

LGTM

Fri, Dec 1, 1:46 PM
efriedma added a comment to D40706: Fix function pointer tail calls in armv8-M.base.

On a sort-of-related note, we currently miscompile the following:

Fri, Dec 1, 1:07 PM
efriedma accepted D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics.

LGTM.

Fri, Dec 1, 12:55 PM
efriedma added inline comments to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Fri, Dec 1, 12:45 PM
efriedma added a comment to D40706: Fix function pointer tail calls in armv8-M.base.

Why are we running out of registers here? We should copy the function pointer to r12, like we do for Thumb2 or ARM.

Fri, Dec 1, 12:36 PM
efriedma added inline comments to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Fri, Dec 1, 11:51 AM
efriedma committed rL319573: [DAGCombine] Simplify ISD::AND handling in ReduceLoadWidth.
[DAGCombine] Simplify ISD::AND handling in ReduceLoadWidth
Fri, Dec 1, 11:34 AM
efriedma closed D40667: [DAGCombine] Simplify ISD::AND handling in ReduceLoadWidth by committing rL319573: [DAGCombine] Simplify ISD::AND handling in ReduceLoadWidth.
Fri, Dec 1, 11:34 AM

Thu, Nov 30

efriedma added inline comments to D40698: [ubsan] Diagnose noreturn functions which return.
Thu, Nov 30, 7:16 PM
efriedma accepted D40695: Improve loop unrolling performance on T99.

Larger than 128 doesn't improve performance or increase unrolling -- at least on SPECcpu2017, libquantum and Google Protobufs, on T99.

Thu, Nov 30, 7:11 PM
efriedma added inline comments to D40698: [ubsan] Diagnose noreturn functions which return.
Thu, Nov 30, 7:07 PM
efriedma added a comment to D40699: Split IndirectBr critical edges before PGO gen/use passes..

The PGO gen/use passes currently fail with an assert failure if there's a critical edge whose source is an IndirectBr instruction.

Thu, Nov 30, 6:34 PM
efriedma added a comment to D40695: Improve loop unrolling performance on T99.

128 is a really big number for LoopMicroOpBufferSize. You might want to consider modifying AArch64TTIImpl::getUnrollingPreferences with some more tailored heuristics.

Thu, Nov 30, 6:28 PM
efriedma added a comment to D33765: Show correct column nr. when multi-byte utf8 chars are used..

Still worried about the effect on tools which parse clang diagnostics... please send a message to cfe-dev. Hopefully we'll get responses there.

Thu, Nov 30, 3:02 PM
efriedma added inline comments to D39758: CodeGen: Fix pointer info in SplitVecOp_EXTRACT_VECTOR_ELT.
Thu, Nov 30, 2:43 PM
efriedma added a comment to D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26.

_Float128 is only *sometimes* the same type as __float128.

Thu, Nov 30, 2:10 PM · Restricted Project
efriedma added inline comments to D40633: [PCG] Poor shuffle lane tracking (PR35454 ).
Thu, Nov 30, 1:06 PM
efriedma added a comment to D39595: [DAGCombine] Refactor ReduceLoadWidth.

See D40667.

Thu, Nov 30, 12:14 PM
efriedma created D40667: [DAGCombine] Simplify ISD::AND handling in ReduceLoadWidth.
Thu, Nov 30, 12:12 PM
efriedma added a comment to D39595: [DAGCombine] Refactor ReduceLoadWidth.

I'm still kind of concerned that this is messier than it needs to be. I guess I'll try experimenting a bit.

Thu, Nov 30, 11:40 AM

Wed, Nov 29

efriedma accepted D40256: [ARM] disable FPU features when using soft floating point..

LGTM

Wed, Nov 29, 12:42 PM
efriedma added a comment to D39627: Fix vtable not receiving hidden visibility when using push(visibility).

You probably just need to pass a "-triple" flag so we don't use Windows mangling or something like that.

Wed, Nov 29, 12:38 PM
efriedma added a comment to D40594: [InstCombine] miscompile of __builtin_fmod.

By many years of precedent, the "frem" instruction is supposed to match the C fmod(), as opposed to something else like the C99 remainder(); probably worth clarifying in LangRef.

Wed, Nov 29, 12:22 PM

Mon, Nov 27

efriedma added inline comments to D40209: [DAGCombiner] eliminate shuffle of insert element.
Mon, Nov 27, 4:57 PM
efriedma added inline comments to D40360: [AArch64][SVE] Asm: Add SVE predicate register definitions and parsing support.
Mon, Nov 27, 3:01 PM
efriedma accepted D38619: [GVN] Prevent ScalarPRE from hoisting across instructions that don't pass control flow to successors.

LGTM

Mon, Nov 27, 2:53 PM
efriedma added a comment to D40480: MemorySSA backed Dead Store Elimination. .

Does this support any transforms which aren't supported by the memdep-based DSE?

Mon, Nov 27, 2:52 PM
efriedma added inline comments to D40256: [ARM] disable FPU features when using soft floating point..
Mon, Nov 27, 2:22 PM
efriedma added a comment to D40209: [DAGCombiner] eliminate shuffle of insert element.

I don't think we need any legality checking because the new insert is identical to the existing insert except that it may have a different constant insertion operand.

Mon, Nov 27, 2:07 PM
efriedma added a comment to D39760: [SimplifyCFG] Teach merge conditional stores to handle cases where the PostBB has more than 2 predecessors by inserting a new block for the store..

Do you know how much impact this has, in the sense of making this transform trigger more often in benchmarks?

Mon, Nov 27, 1:59 PM

Wed, Nov 22

efriedma added a comment to D39352: [SimplifyCFG] Don't do if-conversion if there is a long dependence chain.

Rather than trying to prevent the flattening from happening in the first place, it might make more sense to reverse it later in the pass pipeline. That would be more general, and avoid blocking other optimizations. The x86 backend already does this in some limited cases; see X86CmovConversion.cpp.

Wed, Nov 22, 4:35 PM
efriedma accepted D40320: [NFC] CodeGen: Handle shift amount type in DAGTypeLegalizer::SplitInteger.

LGTM

Wed, Nov 22, 12:26 PM
efriedma added reviewers for D40369: Support sext, zext and trunc instructions in SCEV delinearization algorithm (new revision): zinob, efriedma.

Please post patches with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

Wed, Nov 22, 12:18 PM

Tue, Nov 21

efriedma added inline comments to D40320: [NFC] CodeGen: Handle shift amount type in DAGTypeLegalizer::SplitInteger.
Tue, Nov 21, 3:05 PM
efriedma added inline comments to D40320: [NFC] CodeGen: Handle shift amount type in DAGTypeLegalizer::SplitInteger.
Tue, Nov 21, 2:25 PM
efriedma accepted D40305: [ARM] Fix an off-by-one error when restoring LR for 16-bit Thumb.

LGTM, but please test your patches more carefully in the future.

Tue, Nov 21, 11:07 AM

Mon, Nov 20

efriedma added a comment to D40256: [ARM] disable FPU features when using soft floating point..

Oh, I see, for some silly reason there are actually *three* -mfloat-abi options: hard, soft, and softfp. hard means float instructions and a hard-float calling convention, soft means no floating-point and a soft-float convention, and softfp means float instructions and a soft-float convention. This is probably worth clarifying with a comment.

Mon, Nov 20, 2:04 PM
efriedma added a comment to D40256: [ARM] disable FPU features when using soft floating point..

-mfpu controls what floating-point/vector instructions the compiler generates. -mfloat-abi controls whether floating-point arguments to functions are passed in floating-point registers. These are completely independent, and we need to support using both of them together to generate NEON code with a soft-float ABI. (Android is built like this.)

Mon, Nov 20, 1:06 PM

Fri, Nov 17

efriedma accepted D40130: [InstSimplify] fold and/or of fcmp ord/uno when operand is known nnan.

That said, this transform seems straightforward enough. LGTM.

Fri, Nov 17, 4:53 PM
efriedma added a comment to D40130: [InstSimplify] fold and/or of fcmp ord/uno when operand is known nnan.

From the perspective of canonicalization, it seems kind of weird to have "%x uno %y" rather than "(%x uno 0) | (%y uno 0)". I mean, the former is probably cheaper to lower, but it leads to a lot of special-case logic to handle the equivalency.

Fri, Nov 17, 4:49 PM
efriedma added a comment to D40061: [WIP] [ARM] Make MachineVerifier more strict about terminators.

So now you make all returns (predicated or not) non-analyzable from analyzeBranch's perspective?

Fri, Nov 17, 1:13 PM
efriedma accepted D40184: [LICM] Fix PR35342.

LGTM

Fri, Nov 17, 12:29 PM
efriedma accepted D40150: [LibCallSimplifier] fix pow(x, 0.5) -> sqrt() transforms.

I think this is okay for now; we can clean it up when the backend is fixed.

Fri, Nov 17, 12:28 PM
efriedma accepted D39611: [CodeGen] change const-ness of complex calls.

LGTM

Fri, Nov 17, 12:26 PM
efriedma added inline comments to D40184: [LICM] Fix PR35342.
Fri, Nov 17, 11:54 AM

Thu, Nov 16

efriedma added a comment to D40150: [LibCallSimplifier] fix pow(x, 0.5) -> sqrt() transforms.

This would be a lot simpler if we could disregard errno when we have relaxed FP math, but I'm assuming it's possible to do something like '-ffast-math -fmath-errno' and still expect errno to be set for domain errors?

Thu, Nov 16, 4:25 PM
efriedma accepted D40137: [ARM] 't' asm constraint should accept i32.

LGTM

Thu, Nov 16, 3:14 PM
efriedma added inline comments to D40137: [ARM] 't' asm constraint should accept i32.
Thu, Nov 16, 2:54 PM
efriedma added inline comments to D40144: Implement `std::launder`.
Thu, Nov 16, 12:17 PM
efriedma added inline comments to D40144: Implement `std::launder`.
Thu, Nov 16, 12:11 PM
efriedma added inline comments to D39595: [DAGCombine] Refactor ReduceLoadWidth.
Thu, Nov 16, 11:58 AM
efriedma added inline comments to D40137: [ARM] 't' asm constraint should accept i32.
Thu, Nov 16, 11:22 AM