[PATCH] D52201: [ThinLTO] Write TYPE_IDs for types used in functions imported by aliases

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 19 07:17:50 PDT 2018


tejohnson added a comment.

Can you update description to point out that this is an issue because we will import the alias as a copy of aliasee?

Out of curiosity, what is the failure mode with this bug?



================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:3881
+
+    if (AS->hasAliasee())
+      if (auto *FS = dyn_cast<FunctionSummary>(&AS->getAliasee()))
----------------
We have already invoked AS->getAliasee() above (which would assert if there was no aliasee). So this check shouldn't be needed.


================
Comment at: llvm/test/ThinLTO/X86/cfi-distributed.ll:24
+
+; R1UN: llvm-bcanalyzer -dump %t.out1.index.bc | FileCheck %s --check-prefix=COMBINED
 
----------------
Typo in RUN. But it looks like this RUN shouldn't be here as I don't see any COMBINED checks?


================
Comment at: llvm/test/ThinLTO/X86/cfi-distributed.ll:27
 ; Since @test calls @test2, the latter should be imported here and the
 ; first index file should reference both type ids.
 ; RUN: llvm-dis %t1.o.thinlto.bc -o - | FileCheck %s --check-prefix=INDEX1
----------------
Comment needs updating.


================
Comment at: llvm/test/ThinLTO/X86/cfi-distributed.ll:34
 ; The second index file, corresponding to @test2, should only contain the
 ; typeid for _ZTS1A.
 ; RUN: llvm-dis %t2.o.thinlto.bc -o - | FileCheck %s --check-prefix=INDEX2
----------------
This comment seems wrong - I think it should be "should only contain the typeid for _ZTS1A2" (which is what we're checking) - can you fix that? I think the comment/check will also need an update for your change - it should also contain the typeid for _ZTS1B2, right?


Repository:
  rL LLVM

https://reviews.llvm.org/D52201





More information about the llvm-commits mailing list