[PATCH] D128863: Add switch to use "source_filename" instead of a hash ID for globally promoted local

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I think the summary could give a clearer description of the problem (I'd also use "module hash" vs "hash ID" to be more explicit as to what the hash is). I think instead of:

During LTO a local promoted to a global gets a unique suffix based
partially on a hash ID. This has a problem, because the hash ID is not
the same as the original hash ID. So any tool that's validating changes
will see a change when there isn't one (i.e. the names of the promoted
locals are changed, but it doesn't reflect a real change).

Maybe something like this?:

During LTO a local promoted to a global gets a unique suffix based on
a hash of the module IR. This means that changes in the local's module
can affect the contents in another module that imported it (because the name
of the imported promoted local is changed, but that doesn't reflect a
real change in the importing module). So any tool that's
validating changes to the importing module will see a superficial change.

Instead of using the module hash, ...



================
Comment at: llvm/test/ThinLTO/X86/promote-local-name.ll:20
+
+; The copy in %t1.bc should not be exported/promoted/renamed
+; RUN: llvm-lto -use-source-filename-for-promoted-locals -thinlto-action=promote %t1.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=NOEXPORTSTATIC
----------------
void wrote:
> tejohnson wrote:
> > I don't think there is a need for this testing here (this is behavior tested elsewhere). You should be able to remove promote-local-name-1.ll completely from this test.
> I removed the extra check, but when I tried to remove one of the .ll files it fails. (I low-key don't know what the `llvm-lto` tool with all of its options does.) Can I just leave it as is?
Hmm, that's odd. If you remove that .ll file and remove references to %t1.bc I'm not sure why you would be getting failures. Looks like no symbols from that file are used elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128863