Page MenuHomePhabricator

efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

efriedma added reviewers for D58560: [AST] Set 'MayAlias' instead of 'MustAlias' in AliasSets for PHI nodes (bugzilla bug#36801): asbirlea, chandlerc, john.brawn, sanjoy, reames.

Please read http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface and repost the patch.

Fri, Feb 22, 5:07 PM · Restricted Project
efriedma accepted D57823: [ARM] Make fullfp16 instructions not conditionalisable..

LGTM

Fri, Feb 22, 12:54 PM · Restricted Project
efriedma added inline comments to D56754: Add Support for Creating and Deleting Unicode Files and Directories in Lit.
Fri, Feb 22, 11:59 AM · Restricted Project

Thu, Feb 21

efriedma added inline comments to D56990: Bugfix for Replacement of tied operand of inline asm.
Thu, Feb 21, 5:46 PM · Restricted Project
efriedma added inline comments to D56754: Add Support for Creating and Deleting Unicode Files and Directories in Lit.
Thu, Feb 21, 5:30 PM · Restricted Project
efriedma added inline comments to D58197: [x86] vectorize more cast ops in lowering to avoid register file transfers.
Thu, Feb 21, 4:13 PM · Restricted Project
efriedma added a comment to D58348: [AArch64] Fix for bug 35094 atomicrmw on Armv8.1-A+lse.

Is that up for debate in case this is an edge case that is unlikely to be hit?

Thu, Feb 21, 2:32 PM · Restricted Project
efriedma added a comment to D47735: [DAGCombiner] Create rotates more aggressively.

I think the general approach is still fine. Given we have funnel shifts, we might want to reassociate to form funnel shifts, rather than just rotates, on targets which have native funnel shift instructions. (We'd still want to prefer rotates where possible, I think.)

Thu, Feb 21, 1:26 PM · Restricted Project
efriedma added a comment to D57823: [ARM] Make fullfp16 instructions not conditionalisable..

Do we need special handling for fp16 NEON instructions in Thumb mode? I think the compiler won't predicate them anyway (it's deprecated, and some CPUs have known errata; see ARMBaseInstrInfo::isPredicable), but it might affect the assembler.

Thu, Feb 21, 1:15 PM · Restricted Project
efriedma added a comment to D57321: Fix LexFloatLiteral Lexing.

So I guess overall, there are three fixes here:

Thu, Feb 21, 12:53 PM · Restricted Project
efriedma added a comment to D56005: [RFC] [LLVM] Allocatable Global Register Variables for ARM.

Yes, we should error in the backend, regardless of what clang does, in case some other frontend is missing the appropriate diagnostics.

Thu, Feb 21, 10:26 AM

Wed, Feb 20

efriedma added a comment to D56005: [RFC] [LLVM] Allocatable Global Register Variables for ARM.

Sorry about the delay.

Wed, Feb 20, 4:35 PM
efriedma added inline comments to D57321: Fix LexFloatLiteral Lexing.
Wed, Feb 20, 4:22 PM · Restricted Project
efriedma added a comment to D58348: [AArch64] Fix for bug 35094 atomicrmw on Armv8.1-A+lse.

Are you sure that Bugzilla link is right?

Wed, Feb 20, 4:01 PM · Restricted Project
efriedma added inline comments to D58460: [AArch64] Optimize floating point materialization.
Wed, Feb 20, 11:43 AM · Restricted Project
efriedma added a comment to D58461: [AArch64] Small fix for getIntImmCost.

Needs a testcase.

Wed, Feb 20, 11:34 AM · Restricted Project

Tue, Feb 19

efriedma added a comment to D57820: [AArch64] Use CAS loops for all atomic operations when available..

The atomics documentation is out of date; the implementation was changed to lower "unsupported" atomic operations to __atomic_*& calls.

Tue, Feb 19, 5:12 PM · Restricted Project
efriedma added reviewers for D58391: [TailCallElim] Add tailcall elimination pass to LTO Pipelines: efriedma, tejohnson, mehdi_amini.

What's the general state of the LTO pipeline at this point? PassManagerBuilder::addLTOOptimizationPasses is adding passes in a really weird order (in particular, the placement of the inliner is really strange). Has anyone looked at it recently? Would it be worth killing it off in favor of the ThinLTO pipeline just so we don't have to maintain it?

Tue, Feb 19, 4:13 PM · Restricted Project
efriedma added a comment to D58375: [Clang][NewPM] Disable tests that are broken under new PM.

I can understand why tests that use -O1 or -O2 would produce different results with the new pass manager, but it looks like not all the tests are like that. Do you know why those tests are failing?

Tue, Feb 19, 3:47 PM · Restricted Project
efriedma accepted D58115: [Codegen] Remove dead flags on Physical Defs in machine cse.

It's odd nobody has run into this before, but LGTM with minor nits.

Tue, Feb 19, 3:32 PM · Restricted Project
efriedma added a comment to D58359: [Analysis] fold load of untouched alloca to undef.

Yes, we fold loads from alloca to undef. (In GVN like you mention, but also in mem2reg.) LangRef should state memory allocated with alloca is uninitialized, and that loading from uninitialized memory produces undef; if either of those is missing, patch welcome to fix it.

Tue, Feb 19, 11:27 AM · Restricted Project
efriedma accepted D58281: [ARM] Add some more missing T1 opcodes for the peephole optimisier.

LGTM

Tue, Feb 19, 11:11 AM

Fri, Feb 15

efriedma accepted D58289: [clang] Only provide C11 features in <float.h> starting with C++17.

LGTM

Fri, Feb 15, 6:27 PM · Restricted Project, Restricted Project
efriedma added a comment to D58260: [INLINER] allow inlining of address taken blocks.

I'm concerned that "nocapture" is not sufficient to describe the necessary property here.

Fri, Feb 15, 10:26 AM · Restricted Project

Thu, Feb 14

efriedma added a comment to D56571: [RFC prototype] Implementation of asm-goto support in clang.

I think this is in good shape.

Thu, Feb 14, 11:31 PM
efriedma updated subscribers of D58266: [MC] Sort DWARF FDEs by the associated CIE before emitting them..
Thu, Feb 14, 5:14 PM · Restricted Project
efriedma created D58266: [MC] Sort DWARF FDEs by the associated CIE before emitting them..
Thu, Feb 14, 5:10 PM · Restricted Project
efriedma added a comment to D58210: [SelectionDAGLegalize] Improve promotion of CTLZ.

Generally we should prefer to perform combines in DAGCombine in cases where it's straightforward. (There are a few scattered optimizations in legalization, but mostly for cases that can't simplify after legalization, like libcalls.)

Thu, Feb 14, 2:16 PM

Wed, Feb 13

efriedma added a comment to D58149: [clang] Make sure C99/C11 features in <float.h> are provided in C++11.

Formally, I don't think C11 is a normative reference for C++11 or C++14, only C++17 (see [intro.refs] in the standard).

Wed, Feb 13, 3:50 PM · Restricted Project
efriedma accepted D58120: [Builtins] Treat `bcmp` as a builtin..

LGTM

Wed, Feb 13, 3:35 PM · Restricted Project
efriedma accepted D57833: [ARM] Add some missing thumb1 opcodes to enable peephole optimisation of CMPs.

LGTM

Wed, Feb 13, 12:42 PM · Restricted Project

Tue, Feb 12

efriedma added a comment to D58153: [Driver] Default all Android ARM targets to NEON..

The official documentation still says "Your app must perform runtime detection to confirm that NEON-capable machine code can be run on the target device" (https://developer.android.com/ndk/guides/cpu-arm-neon#runtime_detection). Is that wrong?

Tue, Feb 12, 4:38 PM · Restricted Project, Restricted Project
efriedma added a comment to D57718: [PPC] Adjust the computed branch offset for the possible shorter distance.

Even if you just want to trigger the patched code, the smaller test case still need to be at least 16KB.

Tue, Feb 12, 3:33 PM · Restricted Project
efriedma added a comment to D58120: [Builtins] Treat `bcmp` as a builtin..

This looks essentially fine, but I'd like to see some basic test coverage for the changes to warnings and constant evaluation.

Tue, Feb 12, 3:09 PM · Restricted Project
efriedma added inline comments to D56305: [AArch64] Support reserving arbitrary general purpose registers.
Tue, Feb 12, 2:36 PM · Restricted Project, Restricted Project
efriedma accepted D45355: [SelectionDAG] Fix return calling convention in expansion of ?MULO.

LGTM

Tue, Feb 12, 8:38 AM · Restricted Project

Mon, Feb 11

efriedma added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

Did you mean declare as a target feature in RISCV.td or I misunderstanding something?

Mon, Feb 11, 6:43 PM · Restricted Project
efriedma committed rG806136f8ef1c: [LoopReroll] Fix reroll root legality checking. (authored by efriedma).
[LoopReroll] Fix reroll root legality checking.
Mon, Feb 11, 4:33 PM
efriedma committed rL353779: [LoopReroll] Fix reroll root legality checking..
[LoopReroll] Fix reroll root legality checking.
Mon, Feb 11, 4:33 PM
efriedma closed D56812: [LoopReroll] Fix reroll root legality checking..
Mon, Feb 11, 4:32 PM · Restricted Project
efriedma accepted D58008: [AArch64] Expand v8i8 cttz (PR39729).

(I think we can improve the generated sequences for some of these, but you obviously don't need to do that here.)

Mon, Feb 11, 4:06 PM · Restricted Project
efriedma added a comment to D58017: [DAG] Add SimplifyDemandedBits support for BSWAP/BITREVERSE.

and+rev16/rev32 isn't really any better than rev+lsr; that's fine as far as it goes. But please make sure we have coverage for cases where the zero-extension is free (e.g. the operand is a load, or a zeroext value, or the result of i32 arithmetic).

Mon, Feb 11, 3:38 PM · Restricted Project
efriedma committed rG88fccbdea70d: [Sema] Mark GNU compound literal array init as an rvalue. (authored by efriedma).
[Sema] Mark GNU compound literal array init as an rvalue.
Mon, Feb 11, 2:54 PM
efriedma committed rL353762: [Sema] Mark GNU compound literal array init as an rvalue..
[Sema] Mark GNU compound literal array init as an rvalue.
Mon, Feb 11, 2:54 PM
efriedma committed rC353762: [Sema] Mark GNU compound literal array init as an rvalue..
[Sema] Mark GNU compound literal array init as an rvalue.
Mon, Feb 11, 2:54 PM
efriedma closed D58069: [Sema] Mark GNU compound literal array init as an rvalue..
Mon, Feb 11, 2:54 PM · Restricted Project
efriedma added inline comments to D58069: [Sema] Mark GNU compound literal array init as an rvalue..
Mon, Feb 11, 2:48 PM · Restricted Project
efriedma added inline comments to D57533: lit: support long paths on Windows.
Mon, Feb 11, 1:22 PM · Restricted Project
efriedma added inline comments to D45355: [SelectionDAG] Fix return calling convention in expansion of ?MULO.
Mon, Feb 11, 1:05 PM · Restricted Project
efriedma created D58069: [Sema] Mark GNU compound literal array init as an rvalue..
Mon, Feb 11, 12:38 PM · Restricted Project

Fri, Feb 8

efriedma committed rG041adb0ed0bf: Fix buildbot failure from r353569. (authored by efriedma).
Fix buildbot failure from r353569.
Fri, Feb 8, 6:22 PM
efriedma committed rC353598: Fix buildbot failure from r353569..
Fix buildbot failure from r353569.
Fri, Feb 8, 6:22 PM
efriedma committed rL353598: Fix buildbot failure from r353569..
Fix buildbot failure from r353569.
Fri, Feb 8, 6:22 PM
efriedma added a comment to D57982: [SanitizierCoverage] Avoid splitting critical edges when destination is a basic block containing unreachable.

But since this block itself shouldn't be reachable this is pointless.

Fri, Feb 8, 3:34 PM
efriedma added inline comments to D57533: lit: support long paths on Windows.
Fri, Feb 8, 3:28 PM · Restricted Project
efriedma accepted D57875: [LegalizeTypes] Expand FNEG to bitwise op for IEEE FP types.

Without this patch is the fneg instruction getting turned into a subtraction instruction? I hope not on any platform!

Fri, Feb 8, 3:10 PM · Restricted Project
efriedma committed rG7f98e3c28841: [CodeGen][NFC] Update comments in CGExprConstant.cpp. (authored by efriedma).
[CodeGen][NFC] Update comments in CGExprConstant.cpp.
Fri, Feb 8, 1:38 PM
efriedma committed rC353571: [CodeGen][NFC] Update comments in CGExprConstant.cpp..
[CodeGen][NFC] Update comments in CGExprConstant.cpp.
Fri, Feb 8, 1:37 PM
efriedma committed rL353571: [CodeGen][NFC] Update comments in CGExprConstant.cpp..
[CodeGen][NFC] Update comments in CGExprConstant.cpp.
Fri, Feb 8, 1:37 PM
efriedma closed D57935: [Sema] Make string literal init an rvalue..

https://reviews.llvm.org/rC353569

Fri, Feb 8, 1:24 PM · Restricted Project
efriedma committed rG3bf72d7d64b8: [Sema] Make string literal init an rvalue. (authored by efriedma).
[Sema] Make string literal init an rvalue.
Fri, Feb 8, 1:21 PM
efriedma committed rL353569: [Sema] Make string literal init an rvalue..
[Sema] Make string literal init an rvalue.
Fri, Feb 8, 1:21 PM
efriedma committed rC353569: [Sema] Make string literal init an rvalue..
[Sema] Make string literal init an rvalue.
Fri, Feb 8, 1:21 PM
efriedma accepted D57954: [ARM] LoadStoreOptimizer: reoder limit.

I guess this is fine. LGTM

Fri, Feb 8, 12:14 PM · Restricted Project
efriedma accepted D57955: [ARM] LoadStoreOptimizer: just a clean-up. NFC..

LGTM

Fri, Feb 8, 11:58 AM · Restricted Project

Thu, Feb 7

efriedma created D57935: [Sema] Make string literal init an rvalue..
Thu, Feb 7, 6:02 PM · Restricted Project
efriedma committed rG3189d5f48cae: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg (authored by efriedma).
[COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg
Thu, Feb 7, 5:18 PM
efriedma committed rL353493: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.
[COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg
Thu, Feb 7, 5:17 PM
efriedma committed rC353493: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.
[COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg
Thu, Feb 7, 5:17 PM
efriedma closed D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.
Thu, Feb 7, 5:17 PM · Restricted Project, Restricted Project
efriedma accepted D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.

LGTM. Do you want me to commit this for you?

Thu, Feb 7, 4:57 PM · Restricted Project, Restricted Project
efriedma added a comment to D57862: [AArch64] Fix condition for "high-vector" DUP optimizations..

Yes, it probably makes sense to high vector extracts in some cases. Actually, for some loops, it might even make sense to introduce an extra shuffle outside the loop to avoid a high-vector extract inside the loop.

Thu, Feb 7, 4:29 PM · Restricted Project
efriedma committed rG29c06093012d: [AArch64] Fix condition for "high-vector" DUP optimizations. (authored by efriedma).
[AArch64] Fix condition for "high-vector" DUP optimizations.
Thu, Feb 7, 4:24 PM
efriedma committed rL353486: [AArch64] Fix condition for "high-vector" DUP optimizations..
[AArch64] Fix condition for "high-vector" DUP optimizations.
Thu, Feb 7, 4:23 PM
efriedma closed D57862: [AArch64] Fix condition for "high-vector" DUP optimizations..
Thu, Feb 7, 4:23 PM · Restricted Project
efriedma accepted D57915: [COFF, ARM64] Remove definitions for _byteswap library functions.

I guess we can track inlining separately, if you want to merge this quickly to unblock the Chrome build. LGTM

Thu, Feb 7, 3:47 PM · Restricted Project, Restricted Project
efriedma added a comment to D57915: [COFF, ARM64] Remove definitions for _byteswap library functions.

I did some quick testing with MSVC; apparently it inlines the implementations of these functions when optimizations are on. We definitely want to support inlining these. Since these are commonly used in performance-sensitive code, I'd prefer to implement the required changes to CodeGenFunction::EmitMSVCBuiltinExpr now, rather than chase after weird performance regressions in the future.

Thu, Feb 7, 3:18 PM · Restricted Project, Restricted Project
efriedma added a comment to D57833: [ARM] Add some missing thumb1 opcodes to enable peephole optimisation of CMPs.

I'd like to see testcases for all the possible add/sub opcodes, if it isn't too much work.

Thu, Feb 7, 12:32 PM · Restricted Project
efriedma accepted D57812: [ARM] Add OptMinSize Subtarget feature.

LGTM

Thu, Feb 7, 12:20 PM · Restricted Project
efriedma added inline comments to D56571: [RFC prototype] Implementation of asm-goto support in clang.
Thu, Feb 7, 12:12 PM
efriedma added a comment to D57915: [COFF, ARM64] Remove definitions for _byteswap library functions.

Should clang IR generation be lowering these to bswap calls anyway? Even if the function technically exists in the ucrt, it's going to be pretty slow to call it.

Thu, Feb 7, 12:06 PM · Restricted Project, Restricted Project

Wed, Feb 6

efriedma created D57862: [AArch64] Fix condition for "high-vector" DUP optimizations..
Wed, Feb 6, 4:14 PM · Restricted Project
efriedma added a comment to D57823: [ARM] Make fullfp16 instructions not conditionalisable..

It's sort of weird to have an instruction that has a predicate operand, yet can't be predicated... but I guess if that's the way the ISA is constructed, we should respect it.

Wed, Feb 6, 2:24 PM · Restricted Project
efriedma accepted D57401: [DAGCombiner] fold add/sub with bool operand based on target's boolean contents.

LGTM

Wed, Feb 6, 2:19 PM · Restricted Project
efriedma added inline comments to D57833: [ARM] Add some missing thumb1 opcodes to enable peephole optimisation of CMPs.
Wed, Feb 6, 1:31 PM · Restricted Project
efriedma added inline comments to D57812: [ARM] Add OptMinSize Subtarget feature.
Wed, Feb 6, 9:44 AM · Restricted Project

Tue, Feb 5

efriedma added a comment to D53037: [InstCombine] combine a shuffle and an extract subvector shuffle .

That case is specifically interesting because when it's split, each half is lowered to a different shuffle, and both shuffles are expensive.

Tue, Feb 5, 11:29 AM · Restricted Project

Mon, Feb 4

efriedma added a comment to D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.

In test/CodeGen/arm64-microsoft-status-reg.cpp, you can write something like // CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2:.*]]), then // CHECK-IR-NEXT: store i64 %[[VAR]] on the next line. See also http://llvm.org/docs/CommandGuide/FileCheck.html .

Mon, Feb 4, 4:37 PM · Restricted Project, Restricted Project
efriedma added inline comments to D57321: Fix LexFloatLiteral Lexing.
Mon, Feb 4, 4:20 PM · Restricted Project
efriedma added inline comments to D56571: [RFC prototype] Implementation of asm-goto support in clang.
Mon, Feb 4, 3:49 PM
efriedma accepted D57616: [DAGCombiner] Discard pointer info when combining extract_vector_elt of a vector load when the index isn't constant.

LGTM

Mon, Feb 4, 3:37 PM · Restricted Project
efriedma added inline comments to D56571: [RFC prototype] Implementation of asm-goto support in clang.
Mon, Feb 4, 1:13 PM
efriedma added a comment to D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.

Yes, we should fix CodeGenFunction::EmitAArch64BuiltinExpr to eliminated the unnecessary calls to CreateZext/CreateTrunc. (With this patch, they're no-ops, but better to clean up the code.)

Mon, Feb 4, 1:10 PM · Restricted Project, Restricted Project
efriedma added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

How do we actually want this to work for LTO? Would it make sense to encode this on global variables somehow, to allow different thresholds for different source files?

Mon, Feb 4, 11:09 AM · Restricted Project
efriedma updated subscribers of D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg.

Missing testcase changes. (It should be possible to check that we aren't inserting incorrect truncation/extension operations in the IR.)

Mon, Feb 4, 10:52 AM · Restricted Project, Restricted Project
efriedma added a comment to D57616: [DAGCombiner] Discard pointer info when combining extract_vector_elt of a vector load when the index isn't constant.

In terms of a testcase, can you check the memory operand in MIR?

Mon, Feb 4, 10:49 AM · Restricted Project

Sun, Feb 3

efriedma added a comment to D57635: Add a new intrinsic and attribute to implement `__builtin_va_arg_pack{,_len}` in Clang.

In terms of the general approach, what problem are we really trying to solve? The ability to forward a variadic argument list on to another variadic function doesn't grant any semantic power if you control the implementation; you can always just use a va_list instead (e.g. convert printf to vprintf). In C++, you can also just use a variadic template. And if the goal is just to allow the compiler to emit fortified calls to known library functions, it would be more straightforward to add a flag to implicitly instrument code.

Sun, Feb 3, 12:10 AM · Restricted Project

Sat, Feb 2

efriedma added a comment to D57635: Add a new intrinsic and attribute to implement `__builtin_va_arg_pack{,_len}` in Clang.

Maybe a silly question, but is this actually worth implementing, at this point? The new IR additions have unusual semantics that transforms are continually going to trip over; I'm not convinced you found all the places that currently need checks. And there isn't any existing code we need to be compatible with, I think. If we were designing a frontend extension from scratch, we would probably take a different approach.

Sat, Feb 2, 2:18 PM · Restricted Project

Fri, Feb 1

efriedma accepted D57631: [COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail.

LGTM

Fri, Feb 1, 4:58 PM · Restricted Project, Restricted Project
efriedma added a comment to D56571: [RFC prototype] Implementation of asm-goto support in clang.

Forgot about protected scopes...

Fri, Feb 1, 4:24 PM