- User Since
- Oct 19 2012, 12:57 AM (309 w, 8 h)
Fri, Sep 14
Corrections are always welcome. Please submit an update to this thread (or a new patch) to fix the changes you propose.
If j-s doesn't exists (remember, most of that list came from the ancient days of llvm, so could very easily be completely wrong, but "works"), I all for removing it and making a new (correct) CPU name as the default for armv6k.
Thu, Sep 13
Tue, Sep 11
Thanks Florian! LGTM too.
Thu, Aug 30
Very detailed, using the modern infrastructure and helpful even to those not using clang or wanting to run external benchmarks.
Jul 19 2018
Hi Evandro, looks great, short and simple!
Moved getMemInstAlignment and getMemInstAddressSpace to IR/Instructions.h which already contains similar helpers. Should I rename them to getLoadStoreAlignment and getLoadStoreAddressSpace to be more in line with the existing getLoadStorePointerOperand?
I'll let Florian/Hideki reply about timeframes and strategies, and will just focus on specific items you list.
Jul 18 2018
I like the idea of the interleave analysis to be at a higher ground, but we have to be careful with LV-specific logic and only hoist what it truly generic.
Jun 21 2018
Thanks! I'm happy with Eli is happy. :)
Jun 20 2018
Jun 19 2018
Jun 8 2018
Adding some reviewers + folks on the original review:
Jun 1 2018
May 31 2018
I'm guessing this is a fix for: http://lab.llvm.org:8011/builders/clang-cmake-thumbv8-full-sh/builds/185
May 23 2018
SPEC06 results look too noisy to conclude anything, especially bzip, xalan and povray. Can you find a more stable machine?
May 18 2018
May 15 2018
May 10 2018
We can't add SPEC, as it's commercial. I'm not sure about others, but please make sure they are open source.
May 4 2018
LGTM with the line removed. :)
Nice catch! Sorry for the delay, LGTM.
May 1 2018
Apr 30 2018
This does look like a heavy hammer for a small fix. From the optimisation guides, CSEL and MOV have the same latency/bandwidth and the condition is probably pipelined in anyway.
This seems like an improvement, but we have to be careful with wide variations and little gain. The geomean is almost null but the standard deviation is higher than 75% of the results.
makes sense. thanks!
Apr 29 2018
Apr 25 2018
Do we have similar cost tests for x86, Arm and others? It would be nice to make sure the change to BasicTTIImplBase didn't break other targets' costs.
Apr 24 2018
Thank you! LGTM!
Apr 23 2018
These patches all look very similar, I think they could be all in the same commit? Even being large, may be easier to see how they are similar and do a quick review.
Apr 22 2018
Apr 19 2018
Adding TableGen's code owner.
Apr 17 2018
Adding Diego and Florian. We just had a good chat about this and maybe keeping the headers inside Transform for now would be better for the changes to come. We can move it later when it becomes clearer what the purpose is.
Apr 15 2018
So, I never liked LoopUtils and I agree a lot of it should go to Analysis but having two copies of LoopUtils is bound to confuse people when including the headers.
Minus the whitespace change, LGTM. Thanks!
Minus the whitespace change, LGTM. Thanks!
Mechanical changes, LGTM. Thanks!
Apr 13 2018
Right, pure code movement, and a good clean up at that, LGTM.
Apr 12 2018
Apr 11 2018
Mechanical update, looks good, thanks!
Apr 10 2018
Apr 9 2018
Simple and NFC. LGTM. Thanks!
Indeed, nice cleanup. I think we discussed this earlier and it's a much welcome change, to merge NEON and SVE parsing.
Apr 8 2018
Apr 6 2018
For now, that the shuffle reduction is in loop utils, this patch is fine. But this really ought to be in target transform info.
LTO is something I never considered before in the context of the target parser, but I understand the issues are similar to what the build attributes were trying to solve, so adding more info shouldn't make it worse.
Apr 4 2018
I had seen the first version of this patch and thought it confusing, so refrained from reviewing (too broad context), but this version is much clearer now, and I can see it as straightforward benefit.
Apr 3 2018
Mar 28 2018
Nice! Great for bisecting! I'll let @maxim-kuvyrkov approve, as he's the one taking care of the Buildbots now.
Mar 23 2018
I don't like this being on by default. I get it it uses EXPENSIVE_CHECKS, but still, most developers use Debug/Asserts builds.
Mar 16 2018
Mar 15 2018
Mar 14 2018
The original code was calculating the unroll factor by NumInstructions like:
Mar 10 2018
Can't this be done in the DAGCombine?
Right, now it's clear what it's doing, thanks! I'm happy with it, LGTM.
Mar 9 2018
Right, this makes a lot more sense! :)