[PATCH] D153311: [Attributor] Unify AAMemoryLocation and AAMemoryBehavior
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 20:29:35 PDT 2023
jdoerfert added inline comments.
================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1997
+ bool &UsedAssumedInformation,
+ bool RecurseForSelectAndPHI = true);
----------------
This was accidentally included. Will remove it. Ignore for now.
================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1379
+ }
+ }
return true;
----------------
This was accidentally included. Will remove it. Ignore for now.
================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/byval-2.ll:37
; TUNIT-LABEL: define {{[^@]+}}@test
-; TUNIT-SAME: (ptr nocapture nofree readonly [[X:%.*]]) #[[ATTR0]] {
+; TUNIT-SAME: (ptr nocapture nofree readnone [[X:%.*]]) #[[ATTR1:[0-9]+]] {
; TUNIT-NEXT: entry:
----------------
This is a curious case. I think, the result is sound in some sense, but not as the lang ref is Witten.
`%X` is read (`%tmp2` in TUNIT below, or in the original call to make the `byval` copy), but the result is unused.
The original `byval` argument `%X` is only written by the callee, which makes it `readnone` on this side of the call, conceptually.
All that said, I need to check this in detail and avoid it. Lang ref should say violating read none results in poison, but it says it results in UB.
================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/inalloca.ll:11
define internal i32 @f(ptr inalloca(%struct.ss) %s) {
-; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: read)
+; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite)
; CHECK-LABEL: define {{[^@]+}}@f
----------------
I think we are more conservative wrt. inalloca. Which is fine with me, given that I do not understand it.
================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/variadic.ll:34
+; CGSCC-NEXT: tail call void (ptr, ptr, ptr, ptr, ptr, ...) @callee_t0f(ptr readnone undef, ptr readnone undef, ptr readnone undef, ptr readnone undef, ptr readnone undef, ptr noundef nonnull readnone byval([[STRUCT_TT0:%.*]]) align 8 dereferenceable(16) @t45)
+; CGSCC-NEXT: ret i32 0
;
----------------
I am not sure if the byval should be a forced read. I guess that might solve the problem above as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153311/new/
https://reviews.llvm.org/D153311
More information about the llvm-commits
mailing list