[PATCH] D30980: [GlobalISel][X86] support G_FRAME_INDEX instruction selection.

[Ahmed Bougacha via Phabricator via llvm-commits]

ab added a comment.

Seems reasonable; a couple nits.



================
Comment at: test/CodeGen/X86/GlobalISel/frameIndex-instructionselect.mir:2
+# RUN: llc -mtriple=x86_64-linux-gnu -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=X64
+# RUN: llc -mtriple=i386-linux-gnu   -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=X32
+--- |
----------------
Also test X32 LEA64_32r? and rename the i386 check prefix ;)


================
Comment at: test/CodeGen/X86/GlobalISel/frameIndex-instructionselect.mir:6
+    %ret = add i32 %arg1, %arg1
+    ret i32 %ret
+  }
----------------
FWIW, I found this function more confusing than helpful: it doesn't involve any frame index for x86_64, yet the mir has one.  Maybe make the function an empty void() ?  We still want to teach mir not to require an IR definition someday.

Alternatively, write a test using an alloca?  That should have the same behavior everywhere.


Repository:
  rL LLVM

https://reviews.llvm.org/D30980