[PATCH] D85638: [ARM] Unrestrict Armv8-a IT when at minsize

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added a comment.

Source changes look good to me, just a few test-related points to address.



================
Comment at: llvm/test/CodeGen/ARM/codesize-ifcvt.mir:2
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple=thumbv7 -run-pass=if-converter %s -o - | FileCheck %s
+# RUN: llc -mtriple=thumbv7 -run-pass=if-converter %s -o - | FileCheck %s --check-prefix=V7
+# RUN: llc -mtriple=thumbv8 -run-pass=if-converter %s -o - | FileCheck %s --check-prefix=V8
----------------
NickGuy wrote:
> Could we also have a test for V8 with -Oz? Given that that is what this change effects most
Nit: Shouldn't this be CHECK-V7? I haven't seen any tests that don't follow that convention.


================
Comment at: llvm/test/CodeGen/ARM/codesize-ifcvt.mir:2-3
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple=thumbv7 -run-pass=if-converter %s -o - | FileCheck %s
+# RUN: llc -mtriple=thumbv7 -run-pass=if-converter %s -o - | FileCheck %s --check-prefix=V7
+# RUN: llc -mtriple=thumbv8 -run-pass=if-converter %s -o - | FileCheck %s --check-prefix=V8
 --- |
----------------
Could we also have a test for V8 with -Oz? Given that that is what this change effects most


================
Comment at: llvm/test/CodeGen/ARM/codesize-ifcvt.mir:223-251
+  ; V8:   tTAILJMPdND @extfunc, 14 /* CC::al */, $noreg, implicit $sp, implicit $sp
   ; CHECK-LABEL: name: test_nosize
   ; CHECK: bb.0 (%ir-block.0):
   ; CHECK:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; CHECK:   liveins: $lr, $r7
   ; CHECK:   renamable $r0 = t2MOVi 1, 14 /* CC::al */, $noreg, $noreg
   ; CHECK:   t2CMPri killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
----------------
Is any of this needed now? It looks identical to the V7 block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85638