[PATCH] D139209: [IRMover] Remove UB implying parameter attributes when necessary

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Linker/IRMover.cpp:656
   if (auto *F = dyn_cast<Function>(New)) {
+    if (auto *SF = dyn_cast<Function>(SGV)) {
+      ensureFunctionCompatibility(*F, *SF);
----------------
It seems this can/should only be an issue when the dest module only contains a declaration, and we are only importing a declaration. Should we be checking that F->isDeclaration() (or doing it after that check falls through)? And should we further check that we wouldn't invoke linkGlobalValueBody() further below? Wondering if the change to ensure function attr compatibility is either unnecessary or would be an error in either of those cases.


================
Comment at: llvm/test/Transforms/FunctionImport/attr_fixup.ll:11
+; also removing `noundef` from `@inner`.
+; RUN: opt -module-summary -passes=deadargelim %p/Inputs/attr_fixup.ll -o %t.inputs.attr_fixup.bc
+
----------------
Suggest making Inputs/attr_fixup.ll contain the post DAE code, rather than running that and expecting it will make the necessary change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139209