Removed the HasT2ExtractPack feature and replaced its references with HasDSP. This then allows the Thumb2 extend instructions to be selected for ARMv8M +dsp. These instruction descriptions have also been refactored and more target tests have been added for their isel.
Details
Diff Detail
Event Timeline
lib/Target/ARM/ARMSubtarget.h | ||
---|---|---|
171 | Any reason they needed to be commented out rather than removed? |
lib/Target/ARM/ARMSubtarget.h | ||
---|---|---|
171 | Ah, no! Just something I missed. Thanks. |
This looks mechanically sound to me, and I'm always in favor of removing subtarget features if we don't really need them. That being said, I'd appreciate it if someone that knows more about these extensions gave the final ok.
lib/Target/ARM/ARMInstrThumb2.td | ||
---|---|---|
1145 | You mean Requires DSP, right? | |
test/CodeGen/Thumb2/thumb2-sxt-uxt.ll | ||
39 | You should share the same check prefixes for the similar cases, e.g. CHECK-NO-DSP for cortex-m3 and armv8m, and CHECK-DSP for the others. Then you don't have to repeat yourself in the checks :) Same for the other test files. | |
41 | This is too tight, I think it's better to use CHECK-NOT: sxtab (like you did in the other test files) since you don't want to see sxtab _at all_ (the registers don't matter). | |
test/CodeGen/Thumb2/thumb2-uxt_rot.ll | ||
29 | Same as above, get rid of the registers here. | |
test/CodeGen/Thumb2/thumb2-uxtb.ll | ||
12 | Same as above (in the whole file) |
Hi Diana,
Thanks for the testing tips, I'll make the changes. For the flags I found that no target specified T2Extract without specifying DSP and I only found a single test that referenced it. I also crossed checked the manuals and couldn't see why the extract instructions had been predicated on that specify flag.
Thanks,
sam
I've had a look through the architecture manuals, and I think we do need two different features here.
For example:
- The QADD instruction (currently predicated on FeatureDSP) was introduced by the DSP extension to ARMv5 (aka ARMv5TE)
- The SXTAB instruction (currently predicated on FeatureT2XtPk) was introduced by ARMv6 (but is also optionally available as part of the DSP extension to ARMv7-M)
Therefore, I think we need to keep two separate features. However, we may be able to make FeatureT2XtPk depend on FeatureDSP, as I haven't been able to find any architecture where SXTAB is available without QADD.
Hi Oli,
As far as I can see, instructions such as QADD aren't available for Thumb prior to T2 so predicating these instructions on IsThumb2 and HasDSP should cover them. I think the same is true for SXTAB, the difference between v7M and v7E-M targets is the HasDSP feature and the tests hopefully proved that SXTAB is only selected in the correct cases. Since HasT2ExtractPack was never defined without HasDSP, I can't see how this can break the expected functionality.
cheers
You mean Requires DSP, right?