Page MenuHomePhabricator

dmgreen (Dave Green)
User

Projects

User does not belong to any projects.

User Details

User Since
May 24 2016, 8:35 AM (155 w, 6 d)

Recent Activity

Today

dmgreen accepted D60690: [AArch64] Skip mask checks for masks with an odd number of elements..

LGTM. Thanks for making the fix.

Tue, May 21, 1:32 AM · Restricted Project

Thu, May 16

dmgreen accepted D59879: [ARM][CMSE] Add commandline option and feature macro.

LGTM. With one comment that I will leave to you for what you think is best.

Thu, May 16, 10:01 AM · Restricted Project

Wed, May 15

dmgreen committed rG0582b22f1020: [ARM] Don't use the Machine Scheduler for cortex-m at minsize (authored by dmgreen).
[ARM] Don't use the Machine Scheduler for cortex-m at minsize
Wed, May 15, 5:58 AM
dmgreen committed rL360769: [ARM] Don't use the Machine Scheduler for cortex-m at minsize.
[ARM] Don't use the Machine Scheduler for cortex-m at minsize
Wed, May 15, 5:55 AM
dmgreen closed D61882: [ARM] Don't use the Machine Scheduler for cortex-m at minsize.
Wed, May 15, 5:55 AM · Restricted Project
dmgreen committed rGd2d0f46cd2ae: [ARM] Cortex-M4 schedule (authored by dmgreen).
[ARM] Cortex-M4 schedule
Wed, May 15, 5:40 AM
dmgreen committed rL360768: [ARM] Cortex-M4 schedule.
[ARM] Cortex-M4 schedule
Wed, May 15, 5:39 AM
dmgreen closed D54142: [ARM] Cortex-M4 schedule.
Wed, May 15, 5:39 AM · Restricted Project
dmgreen added a comment to D61882: [ARM] Don't use the Machine Scheduler for cortex-m at minsize.

Sorry guys, I was distracted by some other work.

Wed, May 15, 5:23 AM · Restricted Project

Tue, May 14

dmgreen created D61882: [ARM] Don't use the Machine Scheduler for cortex-m at minsize.
Tue, May 14, 12:49 AM · Restricted Project

Mon, May 6

dmgreen accepted D61169: [CodeGenPrepare] Don't split the store if it is volatile.

This LGTM.

Mon, May 6, 3:20 PM · Restricted Project

Wed, May 1

dmgreen added a comment to D61400: [SelectionDAG] Utilize ARM shift behavior.

I was under the impression that the shift was by the bottom byte amount. i.e the mask is 255, and a shift of 256 is the same as a shift of 0. I have not tried it though.

This is true for NEON vectors, not scalars, as far as I can tell.

Wed, May 1, 3:31 PM · Restricted Project
dmgreen added a comment to D61400: [SelectionDAG] Utilize ARM shift behavior.

I was under the impression that the shift was by the bottom byte amount. i.e the mask is 255, and a shift of 256 is the same as a shift of 0. I hve not tried it though.

Wed, May 1, 2:44 PM · Restricted Project

Tue, Apr 30

dmgreen added a comment to D59535: [SelectionDAG] Compute known bits of CopyFromReg.

Hello. I added a constant address case in rL358114, and a couple of other tests for minsize in rL358128. I was trying to look into X86 tests, but the addressing modes being so extensive makes it difficult. There will possibly be some RISCV testcases out of D60294 too.

Tue, Apr 30, 2:46 PM · Restricted Project

Mon, Apr 29

dmgreen accepted D59787: [ARM] Implement TTI::getMemcpyCost.

Nice. LGTM

Mon, Apr 29, 8:17 AM · Restricted Project

Sun, Apr 28

dmgreen added a comment to D61055: [LoopSimplifyCFG] Suppress expensive DomTree verification in LoopSimplifyCFG.

I claim that it's better to make it obvious that these checks are slow, and make developers either guard them with EXPENSIVE_CHECKS or opt-in with Fast.

Sun, Apr 28, 11:54 PM · Restricted Project
dmgreen added a comment to D61169: [CodeGenPrepare] Don't split the store if it is volatile.

Looks like there is similar code in DAGCombiner that may need a volatile check too, if you search for isMultiStoresCheaperThanBitsMerge.

Sun, Apr 28, 11:09 PM · Restricted Project
dmgreen added a comment to D61055: [LoopSimplifyCFG] Suppress expensive DomTree verification in LoopSimplifyCFG.

That being said, I don't think that people should asserting correctness of domtree within transforms, unless there's a strong need. This is generally not cheap, even at the Fast level, and because of that I think that if someone really wants to have a fast domtree verification in an assertion, they should make it explicit. Historically, Fast had been the default, and the running time penalty started to pile up, as many places assumed it's very cheap to perform. It's much easier to realize these assertions are fishy if they are not usually-cheap-but-not-quite by default.

Sun, Apr 28, 12:46 PM · Restricted Project
dmgreen added a comment to D61055: [LoopSimplifyCFG] Suppress expensive DomTree verification in LoopSimplifyCFG.

Yeah, sounds like a good idea to me. I almost sugested the same thing. There are a load of tests that should still be using Full (i.e. the domtree construction still needs to be checked with basic/full).

Sun, Apr 28, 7:28 AM · Restricted Project
dmgreen added a comment to D61169: [CodeGenPrepare] Don't split the store if it is volatile.

Hello. I feel like this splitting just shouldn't be done for volatile stores.

Sun, Apr 28, 2:08 AM · Restricted Project
dmgreen added a reviewer for D61055: [LoopSimplifyCFG] Suppress expensive DomTree verification in LoopSimplifyCFG: kuhar.

Hello. I think it's fine to always use DominatorTree::VerificationLevel::Fast, the others are more about checking that the DomTree construction code was correct. Fast will compare against a new tree, so for updates will check the the tree is correct.

Sun, Apr 28, 12:26 AM · Restricted Project

Fri, Apr 26

dmgreen added inline comments to D59787: [ARM] Implement TTI::getMemcpyCost.
Fri, Apr 26, 3:18 AM · Restricted Project

Wed, Apr 24

dmgreen added inline comments to D59787: [ARM] Implement TTI::getMemcpyCost.
Wed, Apr 24, 2:01 PM · Restricted Project

Tue, Apr 23

dmgreen added a comment to D60295: [SelectionDAG] Change the jump table size unit from entry to target.

Since there aren't that many jump tables, the increase in code size is negligible.

Tue, Apr 23, 2:37 PM · Restricted Project
dmgreen added a comment to D60294: [DAGCombiner] [CodeGenPrepare] Split large offsets from base addresses.

With rL358845 in, the remaining results I ran look good. Consider us out of your way.

Tue, Apr 23, 1:26 PM · Restricted Project
dmgreen committed rGc519d3c40398: [ARM] Update check for CBZ in Ifcvt (authored by dmgreen).
[ARM] Update check for CBZ in Ifcvt
Tue, Apr 23, 5:12 AM
dmgreen committed rL358977: [ARM] Update check for CBZ in Ifcvt.
[ARM] Update check for CBZ in Ifcvt
Tue, Apr 23, 5:12 AM
dmgreen closed D60090: [ARM] Update check for CBZ in Ifcvt.
Tue, Apr 23, 5:12 AM · Restricted Project
dmgreen added a comment to D60090: [ARM] Update check for CBZ in Ifcvt.

Thanks James!

Tue, Apr 23, 5:07 AM · Restricted Project
dmgreen committed rG2f9eed626532: [ARM] Don't replicate instructions in Ifcvt at minsize (authored by dmgreen).
[ARM] Don't replicate instructions in Ifcvt at minsize
Tue, Apr 23, 4:46 AM
dmgreen committed rL358974: [ARM] Don't replicate instructions in Ifcvt at minsize.
[ARM] Don't replicate instructions in Ifcvt at minsize
Tue, Apr 23, 4:45 AM
dmgreen closed D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.
Tue, Apr 23, 4:45 AM · Restricted Project
dmgreen added a comment to D60090: [ARM] Update check for CBZ in Ifcvt.

ping

Tue, Apr 23, 2:01 AM · Restricted Project
dmgreen added a comment to D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.

Thanks Eli

Tue, Apr 23, 2:01 AM · Restricted Project
dmgreen committed rG63a2aa715ad0: [LSR] Limit the recursion for setup cost (authored by dmgreen).
[LSR] Limit the recursion for setup cost
Tue, Apr 23, 1:51 AM
dmgreen committed rL358958: [LSR] Limit the recursion for setup cost.
[LSR] Limit the recursion for setup cost
Tue, Apr 23, 1:51 AM
dmgreen closed D60944: [LSR] Limit the recursion for setup cost.
Tue, Apr 23, 1:51 AM · Restricted Project

Apr 21 2019

dmgreen committed rG0d741507f7ec: [ARM] Rewrite isLegalT2AddressImmediate (authored by dmgreen).
[ARM] Rewrite isLegalT2AddressImmediate
Apr 21 2019, 2:53 AM
dmgreen committed rL358845: [ARM] Rewrite isLegalT2AddressImmediate.
[ARM] Rewrite isLegalT2AddressImmediate
Apr 21 2019, 2:52 AM
dmgreen closed D60677: [ARM] Rewrite isLegalT2AddressImmediate.
Apr 21 2019, 2:52 AM · Restricted Project
dmgreen added a comment to D60677: [ARM] Rewrite isLegalT2AddressImmediate.

Thanks Tim

Apr 21 2019, 2:49 AM · Restricted Project
dmgreen created D60944: [LSR] Limit the recursion for setup cost.
Apr 21 2019, 2:09 AM · Restricted Project

Apr 17 2019

dmgreen added a comment to D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.

ping

Apr 17 2019, 10:47 AM · Restricted Project
dmgreen added a comment to D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32..

I don't think we here care about auto-updating, not supporting bitcode/lto libraries.

Apr 17 2019, 10:10 AM · Restricted Project, Restricted Project

Apr 16 2019

dmgreen added a comment to D59936: SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259).

OK. Sounds good to me. We can always change it if necessary.

Apr 16 2019, 3:13 AM · Restricted Project

Apr 15 2019

dmgreen added a comment to D59936: SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259).

What are you thinking is best here? I agree that lifetime.end shouldn't prohibit optimisations. Do you think it's best to fix asan and go with this as-is, or to try and prevent sinking if it will only sink a lifetime.time?

Apr 15 2019, 11:14 AM · Restricted Project
dmgreen added a comment to D60704: [ARM] Disallow SP and PC in VMOVRH and VMOVHR..

I agree with this. Does it have a test anywhere?

Apr 15 2019, 10:11 AM · Restricted Project
dmgreen accepted D60705: [ARM] Turn some undefined encoding bits into mandatory 1s..

LGTM

Apr 15 2019, 10:11 AM · Restricted Project
dmgreen added inline comments to D60677: [ARM] Rewrite isLegalT2AddressImmediate.
Apr 15 2019, 9:41 AM · Restricted Project
dmgreen updated the diff for D60677: [ARM] Rewrite isLegalT2AddressImmediate.
Apr 15 2019, 9:41 AM · Restricted Project
dmgreen created D60677: [ARM] Rewrite isLegalT2AddressImmediate.
Apr 15 2019, 1:36 AM · Restricted Project

Apr 11 2019

dmgreen added a comment to D60294: [DAGCombiner] [CodeGenPrepare] Split large offsets from base addresses.

Hello. This looks like an interesting patch. Thanks for working on it. I ran some numbers and on Thumb1 targets (where resources are generally very constrained) this looks like a nice improvement.

Apr 11 2019, 2:59 PM · Restricted Project

Apr 10 2019

dmgreen committed rGdeb33420187f: [ARM] Add an extra test for constant hoist. NFC (authored by dmgreen).
[ARM] Add an extra test for constant hoist. NFC
Apr 10 2019, 12:18 PM
dmgreen committed rL358128: [ARM] Add an extra test for constant hoist. NFC.
[ARM] Add an extra test for constant hoist. NFC
Apr 10 2019, 12:17 PM
dmgreen added a comment to D59535: [SelectionDAG] Compute known bits of CopyFromReg.

Thanks. I've reverted in r358113 and will attempt to add some extra tests in.

Apr 10 2019, 11:11 AM · Restricted Project
dmgreen committed rG4e3fd7757aa7: [ARM] Add an extra constant hoisting test. NFC (authored by dmgreen).
[ARM] Add an extra constant hoisting test. NFC
Apr 10 2019, 11:05 AM
dmgreen committed rL358114: [ARM] Add an extra constant hoisting test. NFC.
[ARM] Add an extra constant hoisting test. NFC
Apr 10 2019, 11:04 AM
dmgreen committed rG0861c87b06c5: Revert rL357745: [SelectionDAG] Compute known bits of CopyFromReg (authored by dmgreen).
Revert rL357745: [SelectionDAG] Compute known bits of CopyFromReg
Apr 10 2019, 10:59 AM
dmgreen committed rL358113: Revert rL357745: [SelectionDAG] Compute known bits of CopyFromReg.
Revert rL357745: [SelectionDAG] Compute known bits of CopyFromReg
Apr 10 2019, 10:59 AM
dmgreen added a reverting change for rL357745: [SelectionDAG] Compute known bits of CopyFromReg: rL358113: Revert rL357745: [SelectionDAG] Compute known bits of CopyFromReg.
Apr 10 2019, 10:59 AM
dmgreen added a comment to D59535: [SelectionDAG] Compute known bits of CopyFromReg.

The patch makes SelectionDAG see more context, so it provides more optimization opportunities.

Apr 10 2019, 3:36 AM · Restricted Project
dmgreen accepted D60427: [ARM] Glue register copies to tail calls..

LGTM then.

Apr 10 2019, 12:40 AM · Restricted Project

Apr 8 2019

dmgreen added a comment to rL357814: Fix PR41395 - __cxa_vec_new may overflow in allocation size calculation..

I'm seeing a bit of an annoying error after this commit. On a native armv7a build, using this command line:

cmake -GNinja /work/llvm/llvm-project/llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLIBCXXABI_USE_LLVM_UNWINDER=ON -DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi;libunwind" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ '-DCMAKE_C_FLAGS= -target armv7a-linux-gnueabihf -march=armv7a' '-DCMAKE_CXX_FLAGS= -target armv7a-linux-gnueabihf -march=armv7a'
ninja cxx

The LIBCXXABI_USE_LLVM_UNWINDER seems to be important. I see an error like:

projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_vector.cpp.o: In function `__cxa_vec_new':
cxa_vector.cpp:(.text.__cxa_vec_new+0x28): undefined reference to `__aeabi_uidiv'
projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_vector.cpp.o: In function `__cxa_vec_new2':
cxa_vector.cpp:(.text.__cxa_vec_new2+0x28): undefined reference to `__aeabi_uidiv'
projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_vector.cpp.o: In function `__cxa_vec_new3':
cxa_vector.cpp:(.text.__cxa_vec_new3+0x28): undefined reference to `__aeabi_uidiv'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

On v7a, the hardware divide was an optional feature so we generate those __aeabi_uidiv calls. I don't know the usual libcxx build process well enough, a lot has changed around here recently. Should we be adding -lgcc somewhere, in order to provide references to these symbols? Or is there a better way to build this? We don't see the same error's elsewhere, I don't think.

Apr 8 2019, 4:44 AM
dmgreen added a comment to D60295: [SelectionDAG] Change the jump table size unit from entry to target.

What does this do for codesize?

Apr 8 2019, 3:28 AM · Restricted Project
dmgreen added a comment to D59535: [SelectionDAG] Compute known bits of CopyFromReg.

We are seeing a lot of codesize increases and performance changes from this patch, unfortunately. Consider this code, which I believe is fairly common in an embedded context:

target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "thumbv6m-arm-none-eabi"
Apr 8 2019, 2:27 AM · Restricted Project

Apr 6 2019

dmgreen added inline comments to D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.
Apr 6 2019, 4:25 PM · Restricted Project
dmgreen updated the diff for D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.
Apr 6 2019, 4:25 PM · Restricted Project
dmgreen updated the diff for D60090: [ARM] Update check for CBZ in Ifcvt.

Refactored

Apr 6 2019, 3:20 PM · Restricted Project

Apr 3 2019

dmgreen added a comment to D59936: SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259).

We saw some good looking codesize decreases from this, by the way. Just to add to the "this patch does good" side of the argument :)

Apr 3 2019, 2:41 AM · Restricted Project

Apr 1 2019

dmgreen created D60090: [ARM] Update check for CBZ in Ifcvt.
Apr 1 2019, 12:24 PM · Restricted Project
dmgreen created D60089: [ARM] Don't replicate instructions in Ifcvt at minsize.
Apr 1 2019, 12:16 PM · Restricted Project
dmgreen accepted D59936: SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259).

Looks like a good improvement to me.

Apr 1 2019, 6:41 AM · Restricted Project

Mar 29 2019

dmgreen added reviewers for D59879: [ARM][CMSE] Add commandline option and feature macro: olista01, christof.
Mar 29 2019, 2:45 AM · Restricted Project
dmgreen added reviewers for D59888: [ARM][CMSE] Add cmse intrinsics for TT instructions: olista01, christof.

Do these need tests? Or does that come later once they do something?

Mar 29 2019, 2:41 AM · Restricted Project

Mar 28 2019

dmgreen accepted D59616: [ARM] Optimize expressions like "return x != 0;" for Thumb1..

LGTM. Thanks.

Mar 28 2019, 3:22 PM · Restricted Project
dmgreen accepted D59385: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz..

LGTM

Mar 28 2019, 3:06 PM · Restricted Project
Herald added a project to D53613: RFC: Explicit Vector Length Intrinsics and Attributes: Restricted Project.
Mar 28 2019, 10:02 AM · Restricted Project

Mar 26 2019

dmgreen accepted D55851: Implement basic loop fusion pass.

Thanks. I had another look through and think this is in a really good state. I'm very happy for it to go in.

Mar 26 2019, 5:51 AM · Restricted Project
dmgreen accepted D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation..

Yeah, OK. This looks like a good patch to me. As I said, I'm not a clang expert, but the code looks sensible enough. (Perhaps wait a couple of days in case others have objections.)

Mar 26 2019, 5:41 AM · Restricted Project, Restricted Project
dmgreen added a comment to D59616: [ARM] Optimize expressions like "return x != 0;" for Thumb1..

Looks like a nice improvement to me.

Mar 26 2019, 4:08 AM · Restricted Project
dmgreen added a reviewer for D59385: [ARM] Don't try to create "push {r12, lr}" in Thumb1 at -Oz.: chill.

I don't pretend to know a huge amount about the frame lowering code, but this looks fine to me.

Mar 26 2019, 3:07 AM · Restricted Project

Mar 24 2019

dmgreen added a comment to D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation..

Hello. I also don't feel very familiar with clang, but had a poke around and I think it looks pretty good. I see unroll and jam is being awkward again.

Mar 24 2019, 11:31 AM · Restricted Project, Restricted Project
dmgreen added a comment to D55851: Implement basic loop fusion pass.

Oh, forgot to mention, I am looking forward to getting this in tree, and think it's very close. If we can sort out the last couple of things here, I think it would be good to get it committed so we can have a play around and sort out some of the questions like where in the pass pipeline it fits.

Mar 24 2019, 7:43 AM · Restricted Project
dmgreen added inline comments to D55851: Implement basic loop fusion pass.
Mar 24 2019, 7:40 AM · Restricted Project

Mar 23 2019

dmgreen accepted D59706: [TTI] Move getIntrinsicCost to allow functions to be overridden. NFC..

From my understanding of TTI, LGTM.

Mar 23 2019, 5:34 AM · Restricted Project

Mar 22 2019

dmgreen accepted D59675: [ARM] [NFC] Use tGPR in patterns where appropriate..

LGTM

Mar 22 2019, 5:04 AM · Restricted Project
dmgreen accepted D59680: [ARM] Don't form "ands" when it isn't scheduled correctly..

LGTM, thanks for putting the fix together.

Mar 22 2019, 4:57 AM · Restricted Project

Mar 21 2019

dmgreen committed rGf0f01051a16d: Fixup opt-remarks.ll gold plugin test. NFC (authored by dmgreen).
Fixup opt-remarks.ll gold plugin test. NFC
Mar 21 2019, 7:35 AM
dmgreen committed rL356669: Fixup opt-remarks.ll gold plugin test. NFC.
Fixup opt-remarks.ll gold plugin test. NFC
Mar 21 2019, 7:35 AM

Mar 17 2019

dmgreen committed rGbaa94ef03bcc: [ARM] Check that CPSR does not have other uses (authored by dmgreen).
[ARM] Check that CPSR does not have other uses
Mar 17 2019, 2:36 PM
dmgreen committed rL356349: [ARM] Check that CPSR does not have other uses.
[ARM] Check that CPSR does not have other uses
Mar 17 2019, 2:35 PM
dmgreen committed rGe0b48a80150d: [ARM] Search backwards for CMP when combining into CBZ (authored by dmgreen).
[ARM] Search backwards for CMP when combining into CBZ
Mar 17 2019, 9:11 AM
dmgreen committed rL356336: [ARM] Search backwards for CMP when combining into CBZ.
[ARM] Search backwards for CMP when combining into CBZ
Mar 17 2019, 9:11 AM
dmgreen closed D59317: [ARM] Search backwards for CMP when combining into CBZ.
Mar 17 2019, 9:11 AM · Restricted Project
dmgreen committed rG30673299d45e: [ARM] Add some CBZ constant island tests. NFC (authored by dmgreen).
[ARM] Add some CBZ constant island tests. NFC
Mar 17 2019, 9:00 AM
dmgreen committed rL356335: [ARM] Add some CBZ constant island tests. NFC.
[ARM] Add some CBZ constant island tests. NFC
Mar 17 2019, 8:59 AM

Mar 14 2019

dmgreen updated the diff for D59317: [ARM] Search backwards for CMP when combining into CBZ.

Removed isARMLowRegister check, reversed the condition of DestOffset - BrOffset, and added some Kill flag handling and tests.

Mar 14 2019, 5:47 AM · Restricted Project
dmgreen added inline comments to D59317: [ARM] Search backwards for CMP when combining into CBZ.
Mar 14 2019, 5:43 AM · Restricted Project
dmgreen updated the diff for D59317: [ARM] Search backwards for CMP when combining into CBZ.

Now with some mir tests.

Mar 14 2019, 3:38 AM · Restricted Project

Mar 13 2019

dmgreen created D59317: [ARM] Search backwards for CMP when combining into CBZ.
Mar 13 2019, 12:46 PM · Restricted Project