[PATCH] D117871: [AArch64][CodeGen] Always use SVE (when enabled) to lower integer divides

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

The code looks fine to me.  For the tests I would prefer it if you removed the new VBITS_EQ_128 lines for the >128bit vector tests as I don't believe they're testing anything new that isn't already being tested by the 64 and 128 bit vector tests.  More than that, because you're hardwiring all the output I can see it just being a source of pain as the tests are more likely to need to be manually updated after unrelated code generation changes, which is something that has previously annoyed people.  If you think the checks offer real value then as a minimum please consider at least removing them from the v32i8 variants as they're likely to be the worst offenders.



================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll:186
+
+; VBITS_EQ_128-LABEL: sdiv_v32i8:
+; VBITS_EQ_128:         ldp q3, q0, [x1]
----------------
Do you need these CHECK lines? I would have thought it's only worth VBITS_EQ_128 CHECk lines for the 128bit tests given I'm sure by now type legalisation is already well tested for NEON.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll:966
+
+; VBITS_EQ_128-LABEL: udiv_v32i8:
+; VBITS_EQ_128:         ldp q3, q0, [x1]
----------------
As above I'm not sure of the value of these CHECK lines.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117871/new/

https://reviews.llvm.org/D117871