[PATCH] D79317: Check type for forward reference definition

[Matthew Parkinson via Phabricator via llvm-commits]

mjp41 created this revision.
mjp41 added reviewers: rriddle, rengolin, theraven, jpienaar, silvas.
mjp41 added a project: MLIR.
Herald added subscribers: Kayjukh, frgossen, grosul1, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, mehdi_amini.
Herald added 1 blocking reviewer(s): rriddle.
Herald added a project: LLVM.
mjp41 removed 1 blocking reviewer(s): rriddle.
Herald added 1 blocking reviewer(s): rriddle.

The types of forward references are checked that they match with other
uses, but they do not check they match with the definition.

  func @forward_reference_type_check() -> (i8) {
    br ^bb2
  
  ^bb1:
    return %1 : i8
  
  ^bb2:
    %1 = "bar"() : () -> (f32)
    br ^bb1
  }

Would be parsed and the use site of '%1' would be silently changed to
'f32'.

This commit adds a test for this case, and a check during parsing for
the types to match.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79317

Files:
  mlir/lib/Parser/Parser.cpp
  mlir/test/IR/invalid.mlir


Index: mlir/test/IR/invalid.mlir
===================================================================
--- mlir/test/IR/invalid.mlir
+++ mlir/test/IR/invalid.mlir
@@ -1496,3 +1496,18 @@
   // expected-error @+1 {{duplicate key in dictionary attribute}}
   "foo.op"() {a, a} : () -> ()
 }
+
+// -----
+
+func @forward_reference_type_check() -> (i8) {
+  br ^bb2
+
+^bb1:
+  // expected-note @+1 {{previously specified type 'i8'}}
+  return %1 : i8
+
+^bb2:
+  // expected-error @+1 {{definition of SSA value '%1#0' has type 'f32'}}
+  %1 = "bar"() : () -> (f32)
+  br ^bb1
+}
Index: mlir/lib/Parser/Parser.cpp
===================================================================
--- mlir/lib/Parser/Parser.cpp
+++ mlir/lib/Parser/Parser.cpp
@@ -3622,6 +3622,15 @@
           .append("previously defined here");
     }
 
+    if (existing.getType() != value.getType())
+    {
+      return emitError(useInfo.loc)
+        .append("definition of SSA value '", useInfo.name, "#", useInfo.number,
+                "' has type ", value.getType())
+        .attachNote(getEncodedSourceLocation(entries[useInfo.number].second))
+        .append("previously specified type ", existing.getType());
+    }
+
     // If it was a forward reference, update everything that used it to use
     // the actual definition instead, delete the forward ref, and remove it
     // from our set of forward references we track.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79317.261761.patch
Type: text/x-patch
Size: 1401 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200504/d4f32046/attachment.bin>