- User Since
- Jan 26 2016, 2:17 AM (176 w, 6 d)
I might be changing my opinion on this patch and won't commit this yet. I will see first how much I actually can reuse from the IT block infrastructure. It would be a bit pointless if I rename this now, and decide later to move the VPT pass in a separate file.
Ha ha, cheers!
Fri, Jun 14
Looks like a useful refactoring to me.
Thanks for your suggestion and explanation Simon. I have shamelessly almost literally copied and added it as a comment as I think that's very useful.
- registered the pass,
- so that I could create and add a MIR test,
- and wrote a range based for-loop.
D62669 includes a few VPT predicable MVE instructions to test the assembly side of this, is that enough to add some MIR tests for this?
Thu, Jun 13
Obviously a test is missing here. I will have a look what is easier: extract a few tablegen instructions descriptions from later patches so that we can create/add a test here, or we can wait for the later patches to be in.
Also, the pass will run, but won't trigger because there are no instructions yet that have getVPTInstrPredicate.
Looks very reasonable to me.
Wed, Jun 12
I can't see what's being changed in ARMTargetTransformInfo.cpp. Can you upload the patch with more context?
Tue, Jun 11
I have enabled this only when we optimise for code-size. The performance results show that there's potential, but as pointed out there is this SPEC regression. But at the moment, we are interested in this patch for code size reasons, and it shows good improvements. I've left a FIXME that it would be worth investigating the regression so that it could be enabled for performance too in a follow up patch.
Mon, Jun 10
Fri, Jun 7
Thanks for reviewing!
Yep, sorry, missed that one.
Cheers, LGTM too.
Set FullFP16 for +mve.fp
Thu, Jun 6
Looks like a good fix to me.
Looks like a good improvement to me.
Wed, Jun 5
Thanks for following up, this one LGTM too.
Thanks for following up. This looks good to me.
Tue, Jun 4
Hi Oliver, thanks for your comments!
Mon, Jun 3
Went through this for the first time, just some initial nits inline, will continue looking a bit.
Fri, May 31
This time with tests.
Ah yes, the school boy error! ;-) Actually, there was a test, but in a different patch; I will move it to here.
This addresses @t.p.northover comment.
Thu, May 30
Many thanks! I will remove those tests before committing!
I've removed the instruction in the tests that are not yet supported; this will be added to another/later patch.
But I haven't done that in this commit, because there was no need to - no Tablegen backend seems to autogenerate default fields in an MCInst.
So what's the test plan here?
Wed, May 29
More targetparser and buildattribute tests!
I am helping Simon a bit with upstreaming.