[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
