efriedma (Eli Friedman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Tue, Aug 15

efriedma added a comment to D34029: Infer lowest bits of an integer Multiply when the low bits of the operands are known.

Sorry, got buried in my inbox. Won't really have time to look again until next week.

Tue, Aug 15, 5:22 PM
efriedma added a comment to D36776: [polly] Fix ScopDetectionDiagnostic test failure caused by r310940.

Please merge the change to XFAIL the test immediately, so the tree isn't broken.

Tue, Aug 15, 5:09 PM · Restricted Project
efriedma added inline comments to D36712: Emit section information for extern variables.
Tue, Aug 15, 4:21 PM
efriedma added inline comments to D36642: [Lexer] Report more precise skipped regions (PR34166).
Tue, Aug 15, 12:20 PM
efriedma accepted D36134: [ARM] Improve loop unrolling for Cortex-M.

LGTM.

Tue, Aug 15, 12:10 PM
efriedma added inline comments to D36762: [Builtins][ARM] Force ARM state for bswap for pre-ARMv6.
Tue, Aug 15, 12:00 PM
efriedma added a comment to D36712: Emit section information for extern variables.

The other side of the problem is, what if the declarations don't have any section information, but the definition does? Is that also an undefined behavior?

Tue, Aug 15, 11:53 AM
efriedma added inline comments to D34598: ScalarEvolution: Add URem support.
Tue, Aug 15, 11:31 AM

Mon, Aug 14

efriedma created D36723: [llvm-cov] Hide instantiation/region coverage from summary tables.
Mon, Aug 14, 5:16 PM
efriedma edited reviewers for D36712: Emit section information for extern variables, added: sdardis, atanasyan, efriedma; removed: eli.friedman.
Mon, Aug 14, 2:55 PM
efriedma added inline comments to D36712: Emit section information for extern variables.
Mon, Aug 14, 2:53 PM
efriedma added a comment to D36642: [Lexer] Report more precise skipped regions (PR34166).

I'm not sure this produces the right locations in general. Consider the following slightly evil testcase:

Mon, Aug 14, 1:22 PM
efriedma added a comment to D36656: [SCCP] Propagate integer range information in IPSCCP (WIP)..

There have been improvements, but we still have a lot of transforms that aren't assume-aware; essentially every call to hasOneUse() does the wrong thing, etc.

Mon, Aug 14, 12:53 PM
efriedma added a comment to D36656: [SCCP] Propagate integer range information in IPSCCP (WIP)..

Should we use range/nonnull metadata or assumptions to generate ranges?

Mon, Aug 14, 12:32 PM

Fri, Aug 11

efriedma accepted D36309: [LoopUnroll] Enable option to unroll remainder loop.

LGTM.

Fri, Aug 11, 2:29 PM
efriedma committed rL310763: [OptDiag] Updating Remarks in SampleProfile.
[OptDiag] Updating Remarks in SampleProfile
Fri, Aug 11, 2:13 PM
efriedma closed D36127: [OptDiag] Updating Remarks in SampleProfile by committing rL310763: [OptDiag] Updating Remarks in SampleProfile.
Fri, Aug 11, 2:13 PM
efriedma added inline comments to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.
Fri, Aug 11, 1:26 PM
efriedma added a comment to D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

For reference, I also ran into this problem at one point, and came up with a hack to just turn off trace-based optimizations for long basic blocks. This implementation looks nicer. :)

Fri, Aug 11, 10:53 AM
efriedma added a comment to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

Okay, that makes sense.

Fri, Aug 11, 10:34 AM

Thu, Aug 10

efriedma updated the diff for D36127: [OptDiag] Updating Remarks in SampleProfile.

Don't emit instruction names for "most popular destination" remarks.

Thu, Aug 10, 6:38 PM
efriedma added a comment to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

This is why I hate DAGCombine; you have to constantly worry about the CSE map invalidating pointers you care about, and I'm never confident I'm handling it correctly.

Thu, Aug 10, 4:28 PM
efriedma added a reviewer for D35156: [ARM] Make ARMExpandPseudo add implicit uses for predicated instructions: qcolombet.

Ping^2

Thu, Aug 10, 3:32 PM
efriedma updated the diff for D36127: [OptDiag] Updating Remarks in SampleProfile.

Address review comments.

Thu, Aug 10, 3:24 PM
efriedma commandeered D36127: [OptDiag] Updating Remarks in SampleProfile.

(Tarun won't have time to finish this, so I'm taking it over.)

Thu, Aug 10, 1:56 PM
efriedma committed rL310653: [ARM] Clarify legal addressing modes for ARM and Thumb2. NFC.
[ARM] Clarify legal addressing modes for ARM and Thumb2. NFC
Thu, Aug 10, 12:32 PM
efriedma closed D36559: [ARM] Clarify legal addressing modes for ARM and Thumb2. NFC by committing rL310653: [ARM] Clarify legal addressing modes for ARM and Thumb2. NFC.
Thu, Aug 10, 12:32 PM

Wed, Aug 9

efriedma accepted D36528: [X86] Keep dependencies when constructing loads in combineStore.

LGTM.

Wed, Aug 9, 5:24 PM
efriedma created D36559: [ARM] Clarify legal addressing modes for ARM and Thumb2. NFC.
Wed, Aug 9, 4:39 PM
efriedma committed rL310518: [llvm-cov] Rearrange entries in report index..
[llvm-cov] Rearrange entries in report index.
Wed, Aug 9, 1:44 PM
efriedma closed D36298: [llvm-cov] Rearrange entries in HTML report index. by committing rL310518: [llvm-cov] Rearrange entries in report index..
Wed, Aug 9, 1:44 PM
efriedma added a comment to D36467: [ARM, FIX] ARMTargetLowering::isLegalAddressingMode can accept illegal addressing modes for Thumb1 target.
$ echo "int a(int x) { return *(int*)(x*2); }" | clang -x c - -S -O2 -mllvm -debug-only=codegenprepare --target=armv7--eabihf -o /dev/null
CGP: Found      local addrmode: [2*%x]
Wed, Aug 9, 12:56 PM
efriedma added inline comments to D36467: [ARM, FIX] ARMTargetLowering::isLegalAddressingMode can accept illegal addressing modes for Thumb1 target.
Wed, Aug 9, 12:26 PM
efriedma added a comment to D36432: [IPSCCP] Add function specialization ability.

It could be the case that specialization enables further propagation, which then enables additional specialization, etc.

Wed, Aug 9, 11:46 AM

Tue, Aug 8

efriedma updated the diff for D36298: [llvm-cov] Rearrange entries in HTML report index..

Update text reports.

Tue, Aug 8, 2:30 PM
efriedma added inline comments to D36487: Emit section information for extern variables. .
Tue, Aug 8, 2:20 PM
efriedma added a comment to D36487: Emit section information for extern variables. .

Please propose a patch for LangRef clarifying what exactly it means to have a section attribute on a declaration.

Tue, Aug 8, 2:11 PM
efriedma added reviewers for D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent).: rengolin, t.p.northover, MatzeB.

Looks fine, but it's a significant change, so adding some other ARM reviewers.

Tue, Aug 8, 2:00 PM
efriedma committed rL310406: [coverage] Special-case calls to noreturn functions..
[coverage] Special-case calls to noreturn functions.
Tue, Aug 8, 1:11 PM
efriedma closed D36250: [coverage] Special-case calls to noreturn functions. by committing rL310406: [coverage] Special-case calls to noreturn functions..
Tue, Aug 8, 1:11 PM
efriedma accepted D36443: [InstCombine] Support pulling left shifts through a subtract with constant LHS.

LGTM, with one small question.

Tue, Aug 8, 1:03 PM
efriedma updated the diff for D36250: [coverage] Special-case calls to noreturn functions..

Update to call VisitStmt(E)

Tue, Aug 8, 12:04 PM
efriedma added inline comments to D36467: [ARM, FIX] ARMTargetLowering::isLegalAddressingMode can accept illegal addressing modes for Thumb1 target.
Tue, Aug 8, 11:29 AM
efriedma added a comment to D36443: [InstCombine] Support pulling left shifts through a subtract with constant LHS.

For reference, http://rise4fun.com/Alive/8l5 .

Tue, Aug 8, 11:13 AM
efriedma added reviewers for D36309: [LoopUnroll] Enable option to unroll remainder loop: efriedma, anna, reames, mkuper, sanjoy, mzolotukhin.
Tue, Aug 8, 11:01 AM
efriedma added a comment to D36432: [IPSCCP] Add function specialization ability.

The way this is written, there isn't much point to sticking the code into IPSCCP; you could just as easily make this a separate pass which runs afterwards. (The only thing you're getting is constant propagation inside the cloned function, which you could just as easily run separately.)

Tue, Aug 8, 10:53 AM

Mon, Aug 7

efriedma added a comment to D36395: [InstCombine] narrow rotate left/right patterns to eliminate zext/trunc (PR34046).

Looks fine, but give it a couple of days to see if anyone else wants to review.

Mon, Aug 7, 5:25 PM
efriedma added inline comments to D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..
Mon, Aug 7, 5:12 PM
efriedma added a comment to D36395: [InstCombine] narrow rotate left/right patterns to eliminate zext/trunc (PR34046).

I spent a bit of time playing with the generated for for various implementations of rotate for 16-bit values. It looks like this transform actually makes things worse; e.g. on ARM the transformed version compiles to one more instruction. Granted, that's probably not important; I think the optimal lowering for a 16-bit rotate on ARM actually involves lowering it to a 32-bit rotate and fixing up the result, and that's probably a transform which doesn't make sense until legalization.

Mon, Aug 7, 12:27 PM
efriedma added a comment to D36309: [LoopUnroll] Enable option to unroll remainder loop.

LoopUnroll has been able to unroll using an upper bound since https://reviews.llvm.org/rL284053 .

Mon, Aug 7, 10:32 AM

Fri, Aug 4

efriedma updated the diff for D36250: [coverage] Special-case calls to noreturn functions..

Add missing call to handleFileExit().

Fri, Aug 4, 5:03 PM
efriedma reopened D36250: [coverage] Special-case calls to noreturn functions..

This was reverted.

Fri, Aug 4, 4:51 PM
efriedma added a comment to D36020: [llvm-cov] Unify region marker placement between text/html modes.

By strange region highlighting, do you mean http://lab.llvm.org:8080/coverage/coverage-reports/all/coverage/Users/buildslave/jenkins/sharedspace/clang-stage2-coverage-R@2/llvm/lib/Target/ARM/ARMISelLowering.cpp.html#L1401 ?

Fri, Aug 4, 4:24 PM
efriedma added a comment to D28335: [WIP] [RFC] Don't lower floating point intrinsics to libcalls which modify errno.

Huh, I guess we do use the x87 sin/cos. It's probably a terrible idea (the microcoded routines are both slow and wildly inaccurate for large inputs), but we do it.

Fri, Aug 4, 12:51 PM
efriedma added a comment to D28335: [WIP] [RFC] Don't lower floating point intrinsics to libcalls which modify errno.

I haven't really been looking at this lately.

Fri, Aug 4, 12:21 PM
efriedma added a comment to D36309: [LoopUnroll] Enable option to unroll remainder loop.

This makes sense.

Fri, Aug 4, 11:24 AM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

What happens if we keep the setRestored() hack, but move the ARM-specific code to call setRestored() on LR into assignCalleeSavedSpillSlots?

Fri, Aug 4, 10:49 AM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

What happens if we keep the setRestored() hack, but move the ARM-specific code to call setRestored() on LR into assignCalleeSavedSpillSlots?

Fri, Aug 4, 10:38 AM

Thu, Aug 3

efriedma created D36298: [llvm-cov] Rearrange entries in HTML report index..
Thu, Aug 3, 6:35 PM
efriedma committed rL309995: [coverage] Special-case calls to noreturn functions..
[coverage] Special-case calls to noreturn functions.
Thu, Aug 3, 3:28 PM
efriedma closed D36250: [coverage] Special-case calls to noreturn functions. by committing rL309995: [coverage] Special-case calls to noreturn functions..
Thu, Aug 3, 3:28 PM
efriedma added a comment to D36229: [X86][Asm] Allow negative immediate to appear before bracketed expression.

Oh, didn't spot the reference to D36230 for the testcase. It would still be nice to add a testcase to the LLVM repo, though (using "llvm-mc -x86-asm-syntax=intel").

Thu, Aug 3, 2:54 PM
efriedma added a comment to D36229: [X86][Asm] Allow negative immediate to appear before bracketed expression.

Needs testcase.

Thu, Aug 3, 2:51 PM
efriedma added a comment to D36287: [SimplifyCFG] Simplify based on a dominating condition (looking past hammocks and diamonds)..

We already have a couple other passes which can do optimize your testcase (JumpThreading and GVN); what do we gain from doing this analysis here specifically?

Thu, Aug 3, 2:48 PM
efriedma added a comment to D36134: [ARM] Improve loop unrolling for Cortex-M.

Okay, I can try to expand on the "limitation of unrolling infrastructure" bit. The question is, given that I have a small loop, and have an expression for the trip count of the loop, what's the optimal way to generate code for the loop?

Thu, Aug 3, 12:59 PM
efriedma added a comment to D35917: [mips] Implement -muninit-const-in-rodata.

LLVM never puts constant data into the BSS. See isSuitableForBSS in lib/Target/TargetLoweringObjectFile.cpp.

Thu, Aug 3, 12:05 PM
efriedma added inline comments to D35917: [mips] Implement -muninit-const-in-rodata.
Thu, Aug 3, 11:51 AM
efriedma added a comment to D35917: [mips] Implement -muninit-const-in-rodata.

Do you actually need a new flag for this? "-fno-common" will ensure clang doesn't generate globals with common linkage.

Thu, Aug 3, 11:46 AM

Wed, Aug 2

efriedma created D36250: [coverage] Special-case calls to noreturn functions..
Wed, Aug 2, 4:40 PM
efriedma committed rL309901: [coverage] Make smaller regions for the first case of a switch..
[coverage] Make smaller regions for the first case of a switch.
Wed, Aug 2, 4:23 PM
efriedma closed D34801: [coverage] Make smaller regions for the first case of a switch. by committing rL309901: [coverage] Make smaller regions for the first case of a switch..
Wed, Aug 2, 4:23 PM
efriedma added a comment to D35584: [CGP] Fold empty dedicated exit blocks created by loopsimplify..

To clarify, the original version of this patch recovered the full 3% regression, but the new version only recovers 0.7%?

Wed, Aug 2, 3:18 PM
efriedma accepted D36110: [COFF, ARM64] Add MS builtins __dmb, __dsb, __isb.

There aren't any useful tests to add here because the generated tables are only used by clang.

Wed, Aug 2, 12:26 PM
efriedma added a comment to D36213: [InstCombine] Remove check for sext of vector icmp from shouldOptimizeCast.

That said, I don't think it makes sense to block this patch on that issue; it's an existing problem.

Wed, Aug 2, 12:20 PM
efriedma added a comment to D35584: [CGP] Fold empty dedicated exit blocks created by loopsimplify..

It's hard to predict what exactly PHIElimination will do at this point in the pipeline... I don't have any better suggestions for a heuristic here.

Wed, Aug 2, 12:16 PM
efriedma added a comment to D36213: [InstCombine] Remove check for sext of vector icmp from shouldOptimizeCast.

Our handling of i1 masks in SelectionDAG is generally terrible.

Wed, Aug 2, 12:04 PM
efriedma added inline comments to D36134: [ARM] Improve loop unrolling for Cortex-M.
Wed, Aug 2, 11:17 AM

Tue, Aug 1

efriedma added a comment to D34801: [coverage] Make smaller regions for the first case of a switch..

I'm going to look over the overall algorithm one more time to make sure this direction makes sense. The current code for ending regions on break/continue/return/noreturn doesn't really work well, and the way we handle multiple consecutive case statments isn't really right, and I want to make sure I'm not going to end up reverting this. And I need to run some tests to make sure I'm not introducing any crashes. If nothing is wrong, I'll commit tomorrow.

Tue, Aug 1, 4:30 PM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

The point I was trying to make is that we should always exclude LR from the set of registers added by addLiveOutsNoPristines: whether or not we actually convert the "bx lr" into a "pop", it isn't live out of the function, and we don't need to model it that way. So assignCalleeSavedSpillSlots wouldn't need any special logic to figure that out.

Tue, Aug 1, 2:09 PM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

Oh, I see what you mean... we don't actually decide whether to "pop {pc}" until ARMFrameLowering::emitPopInst.

Tue, Aug 1, 12:52 PM
efriedma added a comment to D36134: [ARM] Improve loop unrolling for Cortex-M.

Diff is missing context.

Tue, Aug 1, 12:34 PM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

Oh, I see what you mean... we don't actually decide whether to "pop {pc}" until ARMFrameLowering::emitPopInst. Actually, there's another potential problem there: if shrink-wrapping ever allows emitting multiple epilogues (D36109 etc.), we might not make the same decision for each epilogue.

Tue, Aug 1, 12:07 PM
efriedma added a comment to D36152: [ARM] Add registers to debuginfo MIR test cases..

"-start-after=livedebugvalues" appears to work.

Tue, Aug 1, 11:38 AM
efriedma added a comment to D36160: Add "Restored" flag to CalleeSavedInfo.

Fundamentally, the problem with LR on ARM is that it isn't really a callee-save register. We just pretend it's callee-save to avoid explicitly tracking its lifetime throughout the function. Actually, ARM isn't the only architecture that does this; the weird part about ARM is that it has the instruction "pop {pc}" (which loads the return address directly from the stack, without loading it back into LR).

Tue, Aug 1, 11:31 AM

Mon, Jul 31

efriedma committed rL309642: [ScheduleDAG] Don't schedule node with physical register interference.
[ScheduleDAG] Don't schedule node with physical register interference
Mon, Jul 31, 5:29 PM
efriedma closed D33818: [ScheduleDAG] Don't schedule node with physical register interference by committing rL309642: [ScheduleDAG] Don't schedule node with physical register interference.
Mon, Jul 31, 5:29 PM
efriedma added a comment to D36061: [MSP430] Implement multiplication by a constant.

That looks like the code for "a * 0x33", not "a * 0x3333"... but I see your point.

Mon, Jul 31, 4:53 PM
efriedma added a comment to D36061: [MSP430] Implement multiplication by a constant.

Can you please suggest where can I put this function? I've never heard about target-independent utilities.

Mon, Jul 31, 3:14 PM
efriedma added a comment to D36110: [COFF, ARM64] Add MS builtins __dmb, __dsb, __isb.

GCCBuiltin/MSBuiltin emit tables which get used by clang IR generation. See Intrinsic::getIntrinsicForGCCBuiltin/Intrinsic::getIntrinsicForMSBuiltin.

Mon, Jul 31, 2:20 PM
efriedma accepted D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

LGTM

Mon, Jul 31, 12:37 PM

Fri, Jul 28

efriedma added inline comments to D34029: Infer lowest bits of an integer Multiply when the low bits of the operands are known.
Fri, Jul 28, 5:28 PM
efriedma added inline comments to D35821: Have ARM call setBooleanContents(ZeroOrOneBooleanContent)..
Fri, Jul 28, 5:23 PM
efriedma committed rL309457: Fix update_llc_test_checks.py ARM parsing.
Fix update_llc_test_checks.py ARM parsing
Fri, Jul 28, 4:59 PM
efriedma closed D35641: Fix update_llc_test_checks.py ARM parsing by committing rL309457: Fix update_llc_test_checks.py ARM parsing.
Fri, Jul 28, 4:59 PM
efriedma added inline comments to D35584: [CGP] Fold empty dedicated exit blocks created by loopsimplify..
Fri, Jul 28, 2:55 PM
efriedma added a comment to D35887: isKnownNonNull: recognize GEP inbounds as non-null.

isKnownNonZero was added in r124183; isKnownNonNull was originally part of BasicAA, then moved in r151446.

Fri, Jul 28, 12:33 PM

Thu, Jul 27

efriedma accepted D35192: [ARM] Use ADDCARRY / SUBCARRY.

LGTM, with minor comments.

Thu, Jul 27, 11:37 AM
efriedma added inline comments to D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..
Thu, Jul 27, 11:33 AM
efriedma accepted D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode..

LGTM

Thu, Jul 27, 11:19 AM
efriedma accepted D35904: [ARM] Add test to check PCS of ARM ABI runtime floating point helpers.

LGTM

Thu, Jul 27, 11:14 AM