[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @htyu LLVM does not support this. Support for doing that was officially removed about ten years ago when data layout became mandatory, but even prior to that IR was already target-specific, e.g. due to target-specific ABI. I think some parts of MLIR may support this, but certainly nothing does on the LLVM IR level. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
htyu wrote: > @karthik-man LLVM _always_ requires a correct data layout. Yes, that includes > InstCombine. What sort of correct data layout should be used if we are optimization machine-independently? Like cross-compilation with `Clang -O3 -emit-llvm` , but optimized the generated IR for different targets later on. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @karthik-man LLVM *always* requires a correct data layout. Yes, that includes InstCombine. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
karthik-man wrote:
I am debugging a Triton issue
(https://github.com/triton-lang/triton/issues/4060), where an {i32, i32, i32,
i64} struct passed to _vprintf_ is printing the wrong value for the i64. The
issue here seems to be that Triton creates a llvm:Module with a default
DataLayout. In the default layout, i64 abi alignment is 4(DefaultAlignments in
DataLayout.cpp). This causes the optimization in this PR to rewrite the GEP to
the i64 as follows:
`%8 = alloca { i32, i32, i32, i64 }, align 8`
`%12 = getelementptr { i32, i32, i32, i64 }, ptr %8, i32 0, i32 3`
to
`%6 = alloca { i32, i32, i32, i64 }, align 8`
`%9 = getelementptr inbounds i8, ptr %6, i64 12`
But _vprintf_ expects the i64 to be at offset 16.
Is it legal for InstCombine to assume that the DL attached to the Module is the
right DL for the target? In other words, is it ok for InstCombine to do
optimizations that are target-dependent?
https://github.com/llvm/llvm-project/pull/68882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @sgundapa Hm, I think the problem may be that while https://github.com/llvm/llvm-project/pull/90802 removes the limitation on the element types, it's still limited to single-index GEPs, while here there are multiple indices. (Assuming this is related to swapping the GEPs at all, I'm stabbing in the dark here.) https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
sgundapa wrote: > @sgundapa Does #90802 fix the issue you're seeing? Unfortunately no. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @sgundapa Does https://github.com/llvm/llvm-project/pull/90802 fix the issue you're seeing? https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
sgundapa wrote: I've observed a significant regression in one of the AMDGPU benchmarks after applying this patch. The base address calculation within the unrolled loop seems to be the source. I've attached "before.log" and "after.log" files that detail the issue. The modified GEP format, introduced by this patch, doesn't align with the canonical form expected by the "separate-constant-offset-from-gep" pass. Consequently, the "straight line strength reduction" (SLSR) pass cannot optimize the computation. While the intention behind this patch, replicating some "split-gep" pass functionality, is understood, the unintended impact on the SLSR pass is notable. Before I delve into potential solutions, I would greatly appreciate your insights and perspective on this matter.[ [after.log](https://github.com/llvm/llvm-project/files/15299364/after.log) [before.log](https://github.com/llvm/llvm-project/files/15299365/before.log) ](url) https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
Prabhuk wrote: > I am debugging an assertion failure regression in one of our projects that > uses the FlatMap data structure implemented here: > > https://pigweed.googlesource.com/pigweed/pigweed/+/refs/heads/main/pw_containers/public/pw_containers/flat_map.h#180 > > The regression was that the `find` function above failed to return the value > that's stored in the Map for a valid key. > > A bit of debugging and git bisect of LLVM commits pointed me to this commit > [90ba330#diff-0ffb024c6c026c4b93507d08e6d24dccf98980023130781164b71586bc747f2d](https://github.com/llvm/llvm-project/commit/90ba33099cbb17e7c159e9ebc5a512037db99d6d#diff-0ffb024c6c026c4b93507d08e6d24dccf98980023130781164b71586bc747f2d) > as the source of failure. Reverting this commit locally fixes the regression. > > I am working on generating a simple reproducer. This bug is the actual root cause of the failure: https://github.com/llvm/llvm-project/issues/85333 https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
Prabhuk wrote: I am debugging an assertion failure regression in one of our projects that uses the FlatMap data structure implemented here: https://pigweed.googlesource.com/pigweed/pigweed/+/refs/heads/main/pw_containers/public/pw_containers/flat_map.h#180 The regression was that the `find` function above failed to return the value that's stored in the Map for a valid key. A bit of debugging and git bisect of LLVM commits pointed me to this commit https://github.com/llvm/llvm-project/commit/90ba33099cbb17e7c159e9ebc5a512037db99d6d#diff-0ffb024c6c026c4b93507d08e6d24dccf98980023130781164b71586bc747f2d as the source of failure. Reverting this commit locally fixes the regression. I am working on generating a simple reproducer. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @Artem-B I've put up https://github.com/llvm/llvm-project/pull/80983 to address this issue. https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
dtcxzyw wrote: Another example: ``` diff --git a/bench/hermes/optimized/Sorting.cpp.ll b/bench/hermes/optimized/Sorting.cpp.ll index 1a808c47..e03089ca 100644 --- a/bench/hermes/optimized/Sorting.cpp.ll +++ b/bench/hermes/optimized/Sorting.cpp.ll @@ -41,20 +41,22 @@ if.end: ; preds = %entry %call5.i.i.i.i.i.i = tail call noalias noundef nonnull ptr @_Znwm(i64 noundef %mul.i.i.i.i.i.i) #9 store ptr %call5.i.i.i.i.i.i, ptr %index, align 8 %add.ptr.i.i.i = getelementptr inbounds i32, ptr %call5.i.i.i.i.i.i, i64 %conv - %_M_end_of_storage.i.i.i = getelementptr inbounds %"struct.std::_Vector_base>::_Vector_impl_data", ptr %index, i64 0, i32 2 + %_M_end_of_storage.i.i.i = getelementptr inbounds i8, ptr %index, i64 16 store ptr %add.ptr.i.i.i, ptr %_M_end_of_storage.i.i.i, align 8 store i32 0, ptr %call5.i.i.i.i.i.i, align 4 - %incdec.ptr.i.i.i.i.i = getelementptr i32, ptr %call5.i.i.i.i.i.i, i64 1 - %cmp.i.i.i.i.i.i.i = icmp eq i32 %sub, 1 + %incdec.ptr.i.i.i.i.i = getelementptr i8, ptr %call5.i.i.i.i.i.i, i64 4 + %sub.i.i.i.i.i = add nsw i64 %conv, -1 + %cmp.i.i.i.i.i.i.i = icmp eq i64 %sub.i.i.i.i.i, 0 br i1 %cmp.i.i.i.i.i.i.i, label %_ZNSt6vectorIjSaIjEEC2EmRKS0_.exit, label %if.end.i.i.i.i.i.i.i if.end.i.i.i.i.i.i.i: ; preds = %if.end %1 = add nsw i64 %mul.i.i.i.i.i.i, -4 tail call void @llvm.memset.p0.i64(ptr align 4 %incdec.ptr.i.i.i.i.i, i8 0, i64 %1, i1 false) + %add.ptr.i.i.i.i.i.i.i = getelementptr inbounds i32, ptr %incdec.ptr.i.i.i.i.i, i64 %sub.i.i.i.i.i br label %_ZNSt6vectorIjSaIjEEC2EmRKS0_.exit _ZNSt6vectorIjSaIjEEC2EmRKS0_.exit: ; preds = %if.end, %if.end.i.i.i.i.i.i.i - %__first.addr.0.i.i.i.i.i = phi ptr [ %incdec.ptr.i.i.i.i.i, %if.end ], [ %add.ptr.i.i.i, %if.end.i.i.i.i.i.i.i ] + %__first.addr.0.i.i.i.i.i = phi ptr [ %incdec.ptr.i.i.i.i.i, %if.end ], [ %add.ptr.i.i.i.i.i.i.i, %if.end.i.i.i.i.i.i.i ] store ptr %__first.addr.0.i.i.i.i.i, ptr %0, align 8 %cmp116.not = icmp eq i32 %end, %begin br i1 %cmp116.not, label %for.end, label %for.body ``` https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
nikic wrote: @dtcxzyw GitHub can't display the diff, and struggles to clone the repo. Can you share the diffs for just the mentioned files? https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)
https://github.com/fhahn approved this pull request. LGTM, looks like a great first step! Will be interesting to see what kind of regressions this surfaces (if any) https://github.com/llvm/llvm-project/pull/68882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
