[PATCH] D135875: [ARM] Add additional targets to divide tests.

Keith Walker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 14 01:40:32 PDT 2022


keith.walker.arm added inline comments.


================
Comment at: llvm/test/CodeGen/ARM/div.ll:4-5
 ; RUN: llc < %s -mtriple=arm-apple-ios -mcpu=swift        | \
-; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV
+; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV \
+; RUN:                  -check-prefix=CHECK-HWDIV-T2
 ; RUN: llc < %s -mtriple=arm-apple-ios -mcpu=cortex-r4    | \
----------------
nickdesaulniers wrote:
> You can use `-check-prefixes=CHECK,CHECK-HWDIV,CHECK-HWDIV-T2` rather than repeatedly using `-check-prefix=`.
Thanks, I'll update all the run lines to use this feature.


================
Comment at: llvm/test/CodeGen/ARM/div.ll:12
+; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV \
+; RUN:                  -check-prefix=CHECK-HWDIV-T2
 ; RUN: llc < %s -mtriple=arm-none-eabi -mcpu=cortex-a8    | \
----------------
nickdesaulniers wrote:
> should this RUN have a check for CHECK-THUMB as well?
Maybe the name "CHECK-HWDIV-T2" could be improved because the CHECK-HWDIV-T2 case also covers the cases when ARM code is generated (rather than Thumb).

So maybe it should be "CHECK-HWDIV-NOT-T1" or possibly CHECK-HWDIV-ARM-OR-THUMB2 (and in this case change CHECK-HWDIV-T1 to CHECK-HWDIV-THUMB1).

Now I've just written that, I think the latter case is possibly the clearest in showing the intent, so I'm going to go with that unless someone provide a better suggestion.


================
Comment at: llvm/test/CodeGen/ARM/div.ll:149
 ; CHECK-EABI: umull
+; No 32-bit => 64-bit HW multiply instruction
+; CHECK-HWDIV-T1: __udivdi3
----------------
nickdesaulniers wrote:
> divide
Actually in this case multiply is correct.

The test cases @f7 and @f8 for checking for no libcall were added as part of this change https://reviews.llvm.org/D130862 which quote "this urem will be expand by DAGCombiner using multiply by magic constant".   The lack of the necessary multiply instruction causes the optimisation in that change to not apply.


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

https://reviews.llvm.org/D135875



More information about the llvm-commits mailing list