[llvm-branch-commits] [llvm] a082c73 - [WebAssembly] Fix FastISel address calculation bug

[Tom Stellard via llvm-branch-commits]

Author: Thomas Lively
Date: 2020-12-01T13:29:33-08:00
New Revision: a082c730b89fe5e544136ebe6370f452fd2772ee

URL: https://github.com/llvm/llvm-project/commit/a082c730b89fe5e544136ebe6370f452fd2772ee
DIFF: https://github.com/llvm/llvm-project/commit/a082c730b89fe5e544136ebe6370f452fd2772ee.diff

LOG: [WebAssembly] Fix FastISel address calculation bug

Fixes PR47040, in which an assertion was improperly triggered during
FastISel's address computation. The issue was that an `Address` set to
be relative to the FrameIndex with offset zero was incorrectly
considered to have an unset base. When the left hand side of an add
set the Address to be 0 off the FrameIndex, the right side would not
detect that the Address base had already been set and could try to set
the Address to be relative to a register instead, triggering an
assertion.

This patch fixes the issue by explicitly tracking whether an `Address`
has been set rather than interpreting an offset of zero to mean the
`Address` has not been set.

Differential Revision: https://reviews.llvm.org/D85581

(cherry picked from commit cc612c29084e907900ce63ad9031ab573a64e942)

Added: 
    llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll

Modified: 
    llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 8a0092a3f298..c2a0d3e01740 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -58,6 +58,9 @@ class WebAssemblyFastISel final : public FastISel {
       int FI;
     } Base;
 
+    // Whether the base has been determined yet
+    bool IsBaseSet = false;
+
     int64_t Offset = 0;
 
     const GlobalValue *GV = nullptr;
@@ -74,8 +77,9 @@ class WebAssemblyFastISel final : public FastISel {
     bool isFIBase() const { return Kind == FrameIndexBase; }
     void setReg(unsigned Reg) {
       assert(isRegBase() && "Invalid base register access!");
-      assert(Base.Reg == 0 && "Overwriting non-zero register");
+      assert(!IsBaseSet && "Base cannot be reset");
       Base.Reg = Reg;
+      IsBaseSet = true;
     }
     unsigned getReg() const {
       assert(isRegBase() && "Invalid base register access!");
@@ -83,8 +87,9 @@ class WebAssemblyFastISel final : public FastISel {
     }
     void setFI(unsigned FI) {
       assert(isFIBase() && "Invalid base frame index access!");
-      assert(Base.FI == 0 && "Overwriting non-zero frame index");
+      assert(!IsBaseSet && "Base cannot be reset");
       Base.FI = FI;
+      IsBaseSet = true;
     }
     unsigned getFI() const {
       assert(isFIBase() && "Invalid base frame index access!");
@@ -98,13 +103,7 @@ class WebAssemblyFastISel final : public FastISel {
     int64_t getOffset() const { return Offset; }
     void setGlobalValue(const GlobalValue *G) { GV = G; }
     const GlobalValue *getGlobalValue() const { return GV; }
-    bool isSet() const {
-      if (isRegBase()) {
-        return Base.Reg != 0;
-      } else {
-        return Base.FI != 0;
-      }
-    }
+    bool isSet() const { return IsBaseSet; }
   };
 
   /// Keep a pointer to the WebAssemblySubtarget around so that we can make the

diff  --git a/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll b/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
new file mode 100644
index 000000000000..616ce295d7f2
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
@@ -0,0 +1,22 @@
+; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs
+
+; Regression test for PR47040, in which an assertion was improperly
+; triggered during FastISel's address computation. The issue was that
+; an `Address` set to be relative to FrameIndex zero was incorrectly
+; considered to have an unset base. When the left hand side of an add
+; set the Address to have a FrameIndex base of 0, the right side would
+; not detect that the Address base had already been set and could try
+; to set the Address to be relative to a register instead, triggering
+; an assertion.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define i32 @foo() {
+  %stack_addr = alloca i32
+  %stack_i = ptrtoint i32* %stack_addr to i32
+  %added = add i32 %stack_i, undef
+  %added_addr = inttoptr i32 %added to i32*
+  %ret = load i32, i32* %added_addr
+  ret i32 %ret
+}