[PATCH] D85581: [WebAssembly] Fix FastISel address calculation bug

[Thomas Lively via Phabricator via llvm-commits]

tlively created this revision.
tlively added a reviewer: sbc100.
Herald added subscribers: llvm-commits, sunfish, hiraditya, jgravelle-google, dschuff.
Herald added a project: LLVM.
tlively requested review of this revision.
Herald added a subscriber: aheejin.

Fixes PR7040, 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85581

Files:
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/test/CodeGen/WebAssembly/fast-isel-pr7040.ll


Index: llvm/test/CodeGen/WebAssembly/fast-isel-pr7040.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/fast-isel-pr7040.ll
@@ -0,0 +1,22 @@
+; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs
+
+; Regression test for PR7040, 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.
+
+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
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -58,6 +58,9 @@
       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 @@
     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 @@
     }
     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 @@
     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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85581.284113.patch
Type: text/x-patch
Size: 2874 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200808/5e252d29/attachment.bin>