[PATCH] D69748: [Attributor][IPConstantProp] Run the Attributor on IPConstantProp tests

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 11 17:31:36 PST 2019


efriedma added a comment.

Looks generally fine.



================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/2009-09-24-byval-ptr.ll:24
+  %1 = load i32, i32* %0
+; CHECK: %struct.MYstr* @mystr, i{{..}} 0, i32 1
+  %2 = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 0 ; <i8*> [#uses=1]
----------------
This is technically a legal optimization given the test as written, but I don't think you're proving it correctly.  readonly on a function does not imply readonly on its byval arguments.  Try the following variation:

```
define internal i32 @vfu2(%struct.MYstr* byval align 4 %u) nounwind readonly {
entry:
  %z = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 1
  store i32 99, i32* %z, align 4
  %0 = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 1 ; <i32*> [#uses=1]
  %1 = load i32, i32* %0
  %2 = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 0 ; <i8*> [#uses=1]
  %3 = load i8, i8* %2
  %4 = zext i8 %3 to i32
  %5 = add i32 %4, %1
  ret i32 %5
}
```


================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll:48
+  ; The call to this function is removed, so the return value must be zapped
+  ; CHECK: ret i8* null
+  ret i8* null
----------------
Comment isn't consistent with CHECK?


================
Comment at: llvm/test/Transforms/Attributor/IPConstantProp/user-with-multiple-uses.ll:5
+; IPSCCP should propagate the 0 argument, eliminate the switch, and propagate
+; the result.
+
----------------
This is the same test as remove-call-inst.ll?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69748





More information about the llvm-commits mailing list