- User Since
- Feb 24 2015, 1:18 AM (217 w, 9 h)
Most points in review inocoorporated, except for the getRegAllocationHints() -- see inlined comment.
Fri, Apr 19
Don't build a new MI in selectAddSubMux(), but instead update the registers, MCInstrDesc and TiedTo flag. This avoids the need to update SlotIndexes. This is needed to not loose any extra ("regalloc") operands or instruction flags (such as "nsw") . Question: Are there any such flags that should be modified when commutation is performed on a sub and it becomes an add?
Thu, Apr 18
Fri, Apr 5
Thank you Quentin for the review! Patch updated per your suggestions. (I am not sure why LIS and VRM are optional parameters to foldMemoryOperandImpl(). It would have simplified my patch somewhat if they were not, but I instead added a check to see that VRM is available.)
Thu, Apr 4
Tue, Apr 2
Regalloc hints for llc/llh added as well, which handled two regressions.
Wed, Mar 27
Thanks for review! Test case updated per suggestion.
Tue, Mar 26
Mon, Mar 25
Patch refined / improved.
Mar 19 2019
Patch is still experimental, but updated with the latest improvements.
Mar 18 2019
I did something similar for the SystemZ PostRA scheduler. Is this patch taking that into consideration? I have not looked at this in detail, but it would be nice if the SystemZ implementation could use this or at least parts of it...
Mar 16 2019
I think the SystemZ test changes look ok (by replacing the undef operands with an argument the tests are more or less unaffected by this patch), but as usual I will let Uli do the formal approval.
Mar 12 2019
Still INCOMPLETE! SPEC builds, but this patch is still experimental.
Mar 11 2019
Mar 10 2019
Mar 4 2019
The patch as of now is a work in progress (experimental).
Feb 26 2019
Replaced by https://reviews.llvm.org/D58270.
Replaced by https://reviews.llvm.org/D58270.
Thanks for help and review!
Feb 25 2019
I don't think this will make much of a difference compile-time wise, so I'd prefer to go with the version where the code looks simpler ...
I tried this patch on SystemZ / SPEC, and as before this seems to have a relatively very minor impact on the number of files changed (7), and on the performance (seemingly unaffected).
Generally we should prefer to perform combines in DAGCombine in cases where it's straightforward.
Feb 23 2019
Patch updated per review, thanks.
Feb 20 2019
Added handling for fp128 on z14, with some new tests that checks that this is working in fp-const-11.ll. NFC on spec on z14.
Feb 15 2019
Feb 14 2019
Tried this idea, see https://reviews.llvm.org/D58270
Feb 13 2019
Feb 12 2019
Thanks for review. r353867.
Feb 11 2019
It seems you're assuming VGM is always available. I guess there needs to be a check for hasVector() somewhere.
Feb 10 2019
Patch was proven to be incorrect by an assertion failure.
Feb 9 2019
I made a new post for just the handling of scalar FP immediates with VGM: https://reviews.llvm.org/D58003
Feb 8 2019
I think 1a would be the best option, indeed.
Feb 7 2019
This didn't seem to do much good on any benchmarks, so abandoning.
Feb 6 2019
Thanks for review. r353330.
Thanks for review. r353325.
Feb 5 2019
Removed cltz_zero_undef i64 since it is not needed.
The support of VGBM for non-zero FP vectors has been removed, which greatly simplifies the patch. This is NFC on SPEC compared to previous version of patch, except for in one file where the floating point VGBM mask of 0xf0f0 is selected as VGMG insteadof VGBM (1 file, 8 places).
Feb 4 2019
Feb 2 2019
Jan 31 2019
Handle the FP case by building a v16i8 vector instead of using TargetFPConstants (or isFPImmLegal).
Tests using '-cost-model -cost-kind=code-size' in the "direct" way requested in earlier review...
Jan 29 2019
If we actually can load the FP constant using an immediate vector instruction, shouldn't we then return true from TLI.isFPImmLegal? Maybe that's the real underlying problem ...
Thanks for review!
Jan 28 2019
Test updated once again to use a bitcast in the IR also before CodeGenPrepare.
Jan 25 2019
I made a new reduced MIR test case from the original source file and this time included the IR as a comment. Hope that works.
Patch updated per previous comment so that it returns all integer BVNs unmodified, and for FP a new BVN is build with TargetConstantFP operands, to avoid expansion into the constant pool.
Can you explain this further? If you leave the BUILD_VECTOR as-is, where is it pushed into the constant pool?
Jan 24 2019
Jan 23 2019
Thanks for the patch. Could you please reduce the test case a bit further? There are kilobytes of debug info metadata > here which don't appear necessary for reproducing the problem.
To start, I suggest running opt -strip -metarenamer -debugify on the IR input. That should still reproduce the issue.
Ah, that's how you do it! :-)
Jan 22 2019
Thanks for review. r351928.