[PATCH] D67061: [mir-canon][NFCi] Adding opt arg to enable vreg renaming only mode.

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 3 09:35:11 PDT 2019


paquette added inline comments.


================
Comment at: llvm/test/CodeGen/MIR/AArch64/mirCanonCopyCopyProp.mir:1-2
-# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck --check-prefixes=CHECK-FULL %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer -canon-vregs-namer=1 %s | FileCheck --check-prefixes=CHECK-NAMER %s
+
----------------
Even though this test is pretty small, I think it would be better to have an explicit separate test for this.

(Then maybe you can use llvm/utils/update_mir_test_checks.py too, which is a nice time-saver. Not sure if it would work on this though?)

Also, is there any particular reason you don't pass -verify-machineinstrs on these tests? IIRC it always kicks in on debug builds, so it's nice to just always have it to prevent bot failures.


================
Comment at: llvm/test/CodeGen/MIR/AArch64/mirCanonVRegnamer.mir:7-12
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-unknown-linux-gnu"
+  define void @foo() {
+    ret void
+  }
----------------
I don't think you need the IR if you pass in `-mtriple aarch64-unknown-unknown` to the llc line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67061





More information about the llvm-commits mailing list