[PATCH] D144682: Fix miscompilation from MergeFunctions-Inline-SROA-InstCombine optimization sequence

[X via Phabricator via llvm-commits]

dingxiangfei2009 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/FunctionComparator.cpp:222
+  }
+  return 0;
+}
----------------
nikic wrote:
> There is more metadata that applies to loads. I can think of `!align`, `!tbaa`, `!alias.scope`, `!noalias`, `!llvm.mem.parallel.loop.access` and `!llvm.access.group`.
> 
> This is why I suggested to compare all metadata and only skip some white-listed cases like `!dbg`. Otherwise we are likely to miss something here, especially also if new metadata is added in the future.
I see. I have added the generic comparator.

By the way, should I also update the [[ https://llvm.org/docs/LangRef.html#id208 | documentation here ]]? I would do it separately, if you see fit.


================
Comment at: llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll:6
+
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+
----------------
nikic wrote:
> Is this line needed?
The data layout line is removed.


================
Comment at: llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll:10
+
+define void @f1(ptr noalias nocapture noundef sret(%1) dereferenceable(8) %0, ptr noalias nocapture noundef dereferenceable(24) %1) {
+  ; CHECK-LABEL: @f1(
----------------
nikic wrote:
> You should be able to drop all the attributes (noalias etc).
All the attributes are now dropped.


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

https://reviews.llvm.org/D144682