[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment. Herald added a subscriber: StephenFan. The ThinLTO related breakage I mentioned above should be fixed as of https://github.com/llvm/llvm-project/commit/451799bb8261bde52bbfef226d019caf1d82aa42. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment. I'm sorry for the noise. Further investigation has shown that this happens when Rust is doing (thin) LTO, and I don't think this patch can be considered in any way "at fault" here, so this is the last you'll hear from me on the topic here. I don't know whether the fault is with how Rust implements (thin) LTO or something on the LLVM side. Basically, after running the `FunctionImporter` on a module, we end up with a `define available_externally void outer` and a `declare void inner`, presumably coming from different modules. `outer` contains the call to `inner` with `poison`, but the parameter is `noundef` on the declaration on `inner`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment.
I didn't manage to repro with `opt`, so still no compilable IR. I did some more
debugging, though:
- Inside `removeDeadArgumentsFromCallers`, `CB->getCalledFunction()->dump()`
(after the modification) is `define void
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8
%0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr noalias nocapture
nonnull readnone align 1 %2) unnamed_addr #0 personality ptr
@rust_eh_personality !dbg !18034 { ... }`. **Definition, no `noundef`.**
- Inside `callPassesUndefToPassingUndefUBParam`,
`Call.getCalledFunction()->dump()` is `declare void
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8
%0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr noalias noundef
nonnull align 1 %2) unnamed_addr #2`. **Declaration, `noundef` is back.**
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133036/new/
https://reviews.llvm.org/D133036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment.
I've included excerpts from the IR below. It will take me a bit to provide
something compilable. Though you are right, the `noundef` did indeed get
removed from the `call`.
*** IR Dump Before DeadArgumentEliminationPass on [module] ***
; Function Attrs: nonlazybind uwtable
define void
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #0 !dbg
!18135 {
%4 = zext i1 %0 to i8, !dbg !18137
call void
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias
noundef nonnull align 1 %2), !dbg !18137
ret void, !dbg !18138
}
*** IR Dump After DeadArgumentEliminationPass on [module] ***
; Function Attrs: nonlazybind uwtable
define void
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #0 !dbg
!18136 {
%4 = zext i1 %0 to i8, !dbg !18138
call void
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias
nonnull align 1 poison), !dbg !18138
ret void, !dbg !18139
}
*** IR Dump Before InstCombinePass on
_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_ ***
; Function Attrs: nonlazybind uwtable
define available_externally void
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #2 !dbg
!20759 {
%4 = zext i1 %0 to i8, !dbg !20762
call void
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias
nonnull align 1 poison), !dbg !20762
ret void, !dbg !20763
}
*** IR Dump After InstCombinePass on
_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_ ***
; Function Attrs: nonlazybind uwtable
define available_externally void
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #2 !dbg
!20759 {
store i1 true, ptr poison, align 1
ret void, !dbg !20762
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133036/new/
https://reviews.llvm.org/D133036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment. I'm still trying to properly minimize this, but this definitely interacts badly with other optimizations (which triggers the Rust CI failure I mentioned above): - We start with two functions, `outer` and `inner`. `outer` calls `inner`. `outer` has a `noundef` argument that it forwards to `inner` (where the argument is also `noundef`). - `PostOrderFunctionAttrsPass` decides, for both functions, that the argument is `readnone`. The `readnone` attribute is //only// added to the function definitions. The `readnone` attribute is //not// added at the `call` to `inner` from `outer`. - The `DeadArgumentEliminationPass` decides that when `outer` calls `inner` the argument should be `poison`. - This patch sees the `poison` being passed to a `noundef` argument and kills the call. I don't know which component should be considered "at fault" here (or if the `readnone` bit is even relevant). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB
TimNN added a comment. This also broke Rust when compiled at LLVM head: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/13166#0182fb4a-0f2d-4f2e-830f-f62b463b8d48. (I don't know whether this was some existing UB that got only exposed by this patch or not, but wanted to make you aware. Reproduces with 2+ codegen units and a 1+ optimization level). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
