Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2016, 1:07 PM (191 w, 8 h)

Recent Activity

Today

efriedma accepted D77763: [SVE] Change return type of getNumElements to unsigned.

LGTM

Wed, Apr 8, 7:01 PM · Restricted Project
efriedma added a comment to D77767: Prevent stack coloring functions whith setjmp / longjmp.

I'm not sure I understand the problem; can you give a C testcase?

Wed, Apr 8, 7:01 PM · Restricted Project
efriedma added a comment to D77587: [SVE] Add new VectorType subclasses.

Looks right, generally.

Wed, Apr 8, 4:51 PM · Restricted Project, Restricted Project
efriedma added inline comments to D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value.
Wed, Apr 8, 11:57 AM
efriedma accepted D76848: [CodeGen][ARM] Error when writing to specific reserved registers in inline asm.

LGTM

Wed, Apr 8, 11:57 AM · Restricted Project
efriedma accepted D77724: [AArch64][SVE] Refine node definitions for ff & nf loads/stores (NFC).

LGTM. I'm surprised this hasn't caused any issues, though.

Wed, Apr 8, 11:57 AM · Restricted Project
efriedma accepted D77434: [llvm][Codegen] Make `getVectorTypeBreakdownMVT` work with scalable types..

LGTM

Wed, Apr 8, 11:57 AM · Restricted Project

Yesterday

efriedma added a comment to D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs..

Ping

Tue, Apr 7, 7:37 PM · Restricted Project, Restricted Project
efriedma added a comment to D76954: LLVM support for BB-cluster sections.

I'm happy with this at a high level; it's not really any more complicated than BasicBlock sections itself, and the added flexibility makes sense.

Tue, Apr 7, 6:32 PM · Restricted Project
efriedma accepted D77699: [JumpThreading] NFC: Simplify ComputeValueKnownInPredecessorsImpl.

LGTM

Tue, Apr 7, 6:32 PM · Restricted Project
efriedma committed rG565b56a72cff: [NFC] Clean up uses of LoadInst constructor. (authored by efriedma).
[NFC] Clean up uses of LoadInst constructor.
Tue, Apr 7, 4:55 PM
efriedma updated subscribers of D77687: [SelectionDAG] Fix usage of Align constructing MachineMemOperands.
Tue, Apr 7, 4:22 PM · Restricted Project
efriedma added inline comments to D77276: Clean up usages of asserting vector getters in Type.
Tue, Apr 7, 4:22 PM · Restricted Project
efriedma accepted D77266: Clean up usages of asserting vector getters in Type.

LGTM

Tue, Apr 7, 4:22 PM · Restricted Project
efriedma added inline comments to D77587: [SVE] Add new VectorType subclasses.
Tue, Apr 7, 4:22 PM · Restricted Project, Restricted Project
efriedma created D77687: [SelectionDAG] Fix usage of Align constructing MachineMemOperands.
Tue, Apr 7, 3:49 PM · Restricted Project
efriedma added a comment to D76848: [CodeGen][ARM] Error when writing to specific reserved registers in inline asm.

Are you intentionally ignoring clobbers?

Tue, Apr 7, 12:31 PM · Restricted Project
efriedma committed rGe9ac757f79cd: [AArch64] Don't expand memcmp in strict align mode. (authored by efriedma).
[AArch64] Don't expand memcmp in strict align mode.
Tue, Apr 7, 11:24 AM
efriedma closed D77599: [AArch64] Don't expand memcmp in strict align mode..
Tue, Apr 7, 11:23 AM · Restricted Project
efriedma accepted D77396: [LangRef] update text for shufflevector.

LGTM

Tue, Apr 7, 10:51 AM · Restricted Project
efriedma accepted D77631: [SelectionDAG] Make getZeroExtendInReg take a vector VT if the operand VT is a vector..

Being consistent with SIGN_EXTEND_INREG is a good argument. LGTM with clang-format issues fixed.

Tue, Apr 7, 10:50 AM · Restricted Project
efriedma accepted D77651: [IfConversion] Disallow TrueBB == FalseBB for valid diamonds.

LGTM

Tue, Apr 7, 10:50 AM · Restricted Project
efriedma added inline comments to D75661: Remove SequentialType from the type heirarchy..
Tue, Apr 7, 10:50 AM · Restricted Project, Restricted Project
efriedma added inline comments to D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value.
Tue, Apr 7, 10:50 AM

Mon, Apr 6

efriedma updated the diff for D77454: [WIP] LoadInst should store Align, not MaybeAlign..

Rebased. I landed a bunch of the trivial non-functional changes.

Mon, Apr 6, 7:06 PM · Restricted Project
efriedma added inline comments to D77538: [Alignment][NFC] Assume AlignmentFromAssumptions::getNewAlignment is always set..
Mon, Apr 6, 7:06 PM · Restricted Project
efriedma committed rG3f13ee8a0001: [NFC] Modernize misc. uses of Align/MaybeAlign APIs. (authored by efriedma).
[NFC] Modernize misc. uses of Align/MaybeAlign APIs.
Mon, Apr 6, 6:02 PM
efriedma committed rG68b03aee1a15: Remove SequentialType from the type heirarchy. (authored by efriedma).
Remove SequentialType from the type heirarchy.
Mon, Apr 6, 5:29 PM
efriedma closed D75661: Remove SequentialType from the type heirarchy..
Mon, Apr 6, 5:29 PM · Restricted Project, Restricted Project
efriedma accepted D77591: [SveEmitter] Explicitly merge with zero/undef.

LGTM

Mon, Apr 6, 4:55 PM · Restricted Project
efriedma accepted D76680: [SveEmitter] Add immediate checks for lanes and complex imms.

LGTM

Mon, Apr 6, 4:55 PM · Restricted Project
efriedma added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

New diagnostics code looks fine. (I didn't try to review the new section logic closely).

Mon, Apr 6, 4:55 PM · Restricted Project
efriedma added a comment to D77596: [SveEmitter] Add NoOverload flag and builtin for svpfalse.

Would it make sense to generalize getSVEType() to getSVETypeList()? It seems like the current approach won't generalize well once you're dealing with more than one overloaded type (for example, llvm.aarch64.sve.scvtf.nxv8f16.nxv8i16).

Mon, Apr 6, 4:55 PM · Restricted Project
efriedma added a comment to D77593: [SveEmitter] Implement zeroing of false lanes.

Maybe better to emit llvm.aarch64.sve.sel for now, if you're trying to avoid IR operations.

Mon, Apr 6, 4:22 PM · Restricted Project
efriedma added inline comments to D77587: [SVE] Add new VectorType subclasses.
Mon, Apr 6, 4:21 PM · Restricted Project, Restricted Project
efriedma added a comment to D77587: [SVE] Add new VectorType subclasses.

If I'm following correctly, this should apply on its own. Then you're expecting the following API changes:

Mon, Apr 6, 3:49 PM · Restricted Project, Restricted Project
efriedma created D77599: [AArch64] Don't expand memcmp in strict align mode..
Mon, Apr 6, 3:48 PM · Restricted Project
efriedma accepted D76611: [SCCP] Use ranges for predicate info conditions..

That seems fine, yes. LGTM

Mon, Apr 6, 2:43 PM · Restricted Project
efriedma added inline comments to D77523: Add CanonicalizeFreezeInLoops pass.
Mon, Apr 6, 2:10 PM · Restricted Project
efriedma accepted D76678: [SveEmitter] Add range checks for immediates and predicate patterns..

LGTM

Mon, Apr 6, 2:10 PM · Restricted Project
efriedma added inline comments to D76680: [SveEmitter] Add immediate checks for lanes and complex imms.
Mon, Apr 6, 2:10 PM · Restricted Project
efriedma accepted D77273: Clean up usages of asserting vector getters in Type.

LGTM

Mon, Apr 6, 2:10 PM · Restricted Project
efriedma added inline comments to D77276: Clean up usages of asserting vector getters in Type.
Mon, Apr 6, 2:10 PM · Restricted Project
efriedma added a comment to D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value.

You can stick a recursive helper function on RecordType/RecordDecl, if that's the concern.

Mon, Apr 6, 1:04 PM
efriedma added a comment to D68414: [SROA] Enhance AggLoadStoreRewriter to rewrite integer load/store if it covers multi fields in original aggregate.

My biggest concern with this patch is that you're blindly rewriting based on the type of the alloca. That type isn't always reliable, particularly when you're dealing with unions. And even in cases where it is reliable, it might not reflect the actual operations the code ends up using in practice. I'm concerned you'll end up preemptively splitting operations that didn't need to be split, and hurt performance by doing that.

Mon, Apr 6, 1:04 PM · Restricted Project
efriedma added a comment to D77137: [Reassociate] Preserve AAManager and BasicAA analyses.

In general, it's possible for a pass to preserve GlobalsAA, and not BasicAA. BasicAA depends on a number of different analysis passes, like LoopInfo, which might not be updated. GlobalsAA doesn't depend on any local analysis passes, so it's much simpler to preserve.

Mon, Apr 6, 1:04 PM · Restricted Project
efriedma added a comment to D77396: [LangRef] update text for shufflevector.

This version lost the "constant integer or undef values" bit?

Mon, Apr 6, 12:30 PM · Restricted Project
efriedma added a comment to D75362: [InstCombine] Process blocks in RPO.

A depth-first search is enough to ensure some predecessor of every block is visited before that block. So the benefit of RPO is to change that to all (non-loop) predecessors, which I guess helps optimizations involving PHI nodes?

Mon, Apr 6, 12:30 PM · Restricted Project
efriedma accepted D77201: [CodeGen][SelectionDAG] Flip Booleans More Often.

Not sure if you have commit access. If you want me to commit this for you, please let me know how to credit you in the "Author" line of the git commit (name and email).

Mon, Apr 6, 11:57 AM · Restricted Project
efriedma added inline comments to D77523: Add CanonicalizeFreezeInLoops pass.
Mon, Apr 6, 11:57 AM · Restricted Project
efriedma added inline comments to D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value.
Mon, Apr 6, 11:57 AM
efriedma added inline comments to D77258: Clean up usages of asserting vector getters in Type.
Mon, Apr 6, 11:27 AM · Restricted Project
efriedma added a comment to D77454: [WIP] LoadInst should store Align, not MaybeAlign..

If you want to take this over, sure, go for it. :)

Mon, Apr 6, 11:25 AM · Restricted Project

Fri, Apr 3

Herald added a reviewer for D77454: [WIP] LoadInst should store Align, not MaybeAlign.: jdoerfert.
Fri, Apr 3, 11:14 PM · Restricted Project
efriedma committed rG83fa811e5bf5: [clang][opaque pointers] Fix up a bunch of "getType()->getElementType()" (authored by efriedma).
[clang][opaque pointers] Fix up a bunch of "getType()->getElementType()"
Fri, Apr 3, 6:26 PM
efriedma committed rG3e5d671c1910: [polly][opaque pointers] Remove use of deprecated APIs. (authored by efriedma).
[polly][opaque pointers] Remove use of deprecated APIs.
Fri, Apr 3, 6:26 PM
efriedma committed rGb11decc221a6: [clang codegen][opaque pointers] Remove use of deprecated constructor (authored by efriedma).
[clang codegen][opaque pointers] Remove use of deprecated constructor
Fri, Apr 3, 6:26 PM
efriedma committed rG501ec31b5960: [llvm-stress][opaque pointers] Remove use of deprecated constructor (authored by efriedma).
[llvm-stress][opaque pointers] Remove use of deprecated constructor
Fri, Apr 3, 6:26 PM
efriedma updated the diff for D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs..

Commited the non-header changes separately

Fri, Apr 3, 6:26 PM · Restricted Project, Restricted Project
efriedma accepted D77425: [NFC] Make Type::isVectorTy call isa<VectorType>.

LGTM

Fri, Apr 3, 6:26 PM · Restricted Project
efriedma accepted D77269: Clean up usages of asserting vector getters in Type.

LGTM

Fri, Apr 3, 5:53 PM · Restricted Project
efriedma added inline comments to D77276: Clean up usages of asserting vector getters in Type.
Fri, Apr 3, 5:53 PM · Restricted Project
efriedma added a comment to D77442: [SVE] Take constant fold fast path for splatted vscale vectors.

Please add test coverage in llvm/test/Analysis/ConstantFolding/

Fri, Apr 3, 5:53 PM · Restricted Project
efriedma added inline comments to D77273: Clean up usages of asserting vector getters in Type.
Fri, Apr 3, 5:53 PM · Restricted Project
efriedma accepted D76961: [SelectionDAG] fix predecessor list for INLINEASM_BRs' parent.

LGTM

Fri, Apr 3, 5:53 PM · Restricted Project
efriedma added inline comments to D77434: [llvm][Codegen] Make `getVectorTypeBreakdownMVT` work with scalable types..
Fri, Apr 3, 4:17 PM · Restricted Project
efriedma added inline comments to D76961: [SelectionDAG] fix predecessor list for INLINEASM_BRs' parent.
Fri, Apr 3, 4:16 PM · Restricted Project
efriedma added inline comments to D77276: Clean up usages of asserting vector getters in Type.
Fri, Apr 3, 2:38 PM · Restricted Project
efriedma accepted D77275: Clean up usages of asserting vector getters in Type.

LGTM

Fri, Apr 3, 2:05 PM · Restricted Project
efriedma accepted D77271: Clean up usages of asserting vector getters in Type.

LGTM

Fri, Apr 3, 2:05 PM · Restricted Project
efriedma added inline comments to D77269: Clean up usages of asserting vector getters in Type.
Fri, Apr 3, 1:36 PM · Restricted Project
efriedma edited reviewers for D77265: Clean up usages of asserting vector getters in Type, added: jonpa; removed: jnspaulsson.
Fri, Apr 3, 1:33 PM · Restricted Project
efriedma added inline comments to D77259: Clean up usages of asserting vector getters in Type.
Fri, Apr 3, 1:33 PM · Restricted Project
efriedma added inline comments to D76961: [SelectionDAG] fix predecessor list for INLINEASM_BRs' parent.
Fri, Apr 3, 1:33 PM · Restricted Project
efriedma added a comment to D77425: [NFC] Make Type::isVectorTy call isa<VectorType>.

This will fail to compile if someone includes Type.h without including DerivedTypes.h.

Fri, Apr 3, 1:33 PM · Restricted Project
efriedma accepted D77317: [llvm][CodeGen] Avoid implicit cast of TypeSize to integer in `initActions`..

LGTM

Fri, Apr 3, 1:33 PM · Restricted Project
efriedma added inline comments to D76866: Enable new passmanager plugin support for LTO..
Fri, Apr 3, 12:25 PM · Restricted Project
efriedma updated the diff for D76866: Enable new passmanager plugin support for LTO..

Fix review comment and lint

Fri, Apr 3, 12:25 PM · Restricted Project
efriedma added inline comments to D76866: Enable new passmanager plugin support for LTO..
Fri, Apr 3, 11:55 AM · Restricted Project
efriedma added inline comments to D77201: [CodeGen][SelectionDAG] Flip Booleans More Often.
Fri, Apr 3, 11:53 AM · Restricted Project
efriedma accepted D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract.

LGTM

Fri, Apr 3, 11:53 AM · Restricted Project
efriedma accepted D77267: Clean up usages of asserting vector getters in Type.

LGTM with one minor suggestion.

Fri, Apr 3, 11:53 AM · Restricted Project
efriedma added a comment to D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value.

Diagnosing call/return statements seems nicer.

Fri, Apr 3, 11:53 AM
efriedma added a comment to D77396: [LangRef] update text for shufflevector.

Mentioning "UndefMaskElem" probably doesn't make sense. Maybe should be "undef"?

Fri, Apr 3, 11:53 AM · Restricted Project

Thu, Apr 2

efriedma updated the diff for D76866: Enable new passmanager plugin support for LTO..

Added support/tests for dynamically loaded NewPM LTO plugins. (Statically linked plugins can't be tested in regression tests because there are none by default.)

Thu, Apr 2, 6:27 PM · Restricted Project
efriedma accepted D77335: [AST] clang::VectorType supports any size (that fits in unsigned).

LGTM

Thu, Apr 2, 5:55 PM · Restricted Project
efriedma added a comment to D76866: Enable new passmanager plugin support for LTO..

The current version only supports static plugins. Supporting dynamic plugins is a more difficult puzzle because I'd have to figure out how to thread the option through... I'll give it a shot, I guess.

Thu, Apr 2, 3:11 PM · Restricted Project
efriedma added a comment to D77335: [AST] clang::VectorType supports any size (that fits in unsigned).

If you're going to add code to check for it, we might as well add testcases for ridiculous sizes, like (__int128_t)1 << 100.

Thu, Apr 2, 2:38 PM · Restricted Project
efriedma added a comment to D76866: Enable new passmanager plugin support for LTO..

Ping

Thu, Apr 2, 12:28 PM · Restricted Project
efriedma added a comment to D77317: [llvm][CodeGen] Avoid implicit cast of TypeSize to integer in `initActions`..

The extra assertion doesn't seem helpful; the one in getFixedSize() should be enough.

Thu, Apr 2, 12:28 PM · Restricted Project
efriedma added a comment to D77201: [CodeGen][SelectionDAG] Flip Booleans More Often.

VT.getScalarSizeInBits(), I think.

Thu, Apr 2, 12:28 PM · Restricted Project
efriedma added a comment to D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract.

You should be able to refactor the patterns into the definitions of the multiclasses sve_int_bin_cons_arit_0 and sve_int_arith_imm0, to avoid repeating them four times. (You might want to look at other places using null_frag in SVEInstrFormats.td for inspiration.)

Thu, Apr 2, 11:55 AM · Restricted Project
efriedma added a comment to D77319: [DAGCombine] Remove the getNegatibleCost to avoid the out of sync with getNegatedExpression.

Can you split whatever change affects the PPC tests into a separate patch, if it's not too tricky?

Thu, Apr 2, 11:55 AM · Restricted Project
efriedma added a comment to D77313: [AST] Allow VectorTypes of 1-256 elements, and powers of two up to 2**31..

I think I would rather just pay the extra 8 bytes per VectorType, and expand this to support all vector types supported by LLVM. It's not like we allocate enough VectorTypes for it to matter, anyway.

Thu, Apr 2, 11:22 AM · Restricted Project
efriedma added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Thu, Apr 2, 8:38 AM · Restricted Project
efriedma added a comment to D77201: [CodeGen][SelectionDAG] Flip Booleans More Often.

That's the right idea. getNumValues() seems wrong, though.

Thu, Apr 2, 7:33 AM · Restricted Project

Wed, Apr 1

efriedma added a comment to D77236: [SVE] Cleanup prior to introdution of FixedVectorType.

My thoughts, in no particular order:

Wed, Apr 1, 2:45 PM · Restricted Project, Restricted Project
efriedma added a comment to D76611: [SCCP] Use ranges for predicate info conditions..

Yes, code in other passes, particularly SimplifyCFG.

Wed, Apr 1, 2:38 PM · Restricted Project
efriedma added a comment to D77201: [CodeGen][SelectionDAG] Flip Booleans More Often.

We have to call getBooleanContents on the vector type, not the vector element type. On ARM, a boolean in a vector is ZeroOrNegativeOneBooleanContent. A scalar boolean on its own is ZeroOrOneBooleanContent. If you choose incorrectly, that's a potential miscompile. (Not sure why it isn't showing up as a regression test change for other targets; maybe the specific VSELECT pattern in question is rare after legalization.)

Wed, Apr 1, 2:04 PM · Restricted Project
efriedma accepted D77131: [clang] Move branch-protection from CodeGenOptions to LangOptions.

LGTM

Wed, Apr 1, 2:04 PM · Restricted Project