[PATCH] D75421: [AArch64][GlobalISel] Avoid copies to target register bank for subregister copies

Raul Tambre via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 4 12:21:36 PST 2020


tambre added a comment.

Thanks for the review Jessica! I addressed your comments, except for being unable to reduce the testcase much further.
Note that this change is a fix for bug 45029 <https://bugs.llvm.org/show_bug.cgi?id=45029>.



================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/subreg-copy.mir:8-18
+  define void @test_128_fpr_truncation() #0 {
+  entry:
+    %0 = load i128, i128* @a, align 16
+    %conv = trunc i128 %0 to i64
+    %and = and i64 %conv, 1
+    %tobool = icmp ne i64 %and, 0
+    br i1 %tobool, label %end, label %end
----------------
paquette wrote:
> The actual IR here isn't necessary; it can probably just be
> 
> ```
> define void @test_128_fpr_truncation() { ret void }
> ```
It seemed to me that the IR was usually included for reference. Seems not! :)


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/subreg-copy.mir:43-49
+    %4:gpr(s64) = G_AND %8:gpr, %3:gpr
+    %7:gpr(s32) = G_ICMP intpred(ne), %4:gpr(s64), %5:gpr
+    %6:gpr(s1) = G_TRUNC %7:gpr(s32)
+    G_BRCOND %6:gpr(s1), %bb.1
+
+  bb.1:
+    RET_ReallyLR
----------------
paquette wrote:
> Is anything after the `COPY` necessary to test the behaviour here? Can we just return the result of the COPY? E.g.
> 
> ```
> $x1 = COPY %8:gpr(s64)
> RET_ReallyLR implicit $x1
> ```
Changing anything after the `COPY` results in the testcase no longer crashing prior to this change. I tried reducing in various ways, but unfortunately no dice.


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

https://reviews.llvm.org/D75421





More information about the llvm-commits mailing list