tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/test/Linker/stack-alignment.ll:11
+;--- main.ll
+; NONE: error: linking module flags 'override-stack-alignment': IDs have 
conflicting values
+; CHECK-16: error: linking module flags 'override-stack-alignment': IDs have 
conflicting values
----------------
nickdesaulniers wrote:
> tejohnson wrote:
> > Will you get this error currently? I thought per comment and behavior of 
> > Error that it shouldn't give an error.
> the `RUN` line for that check is missing the `:`, so it's not actually run.  
> I think I'll create the new `ModFlagBehavior` as a child revision to this 
> commit, in which I add the new behavior and upgrade this attribute to use it, 
> adding in this unit test.
> 
> ---
> 
> Now that I've had time to play with implementing such a `ModFlagBehavior`; it 
> looks like it's not possible to create a module with such semantics then test 
> it properly with `llvm-link`.  The reason is that `llvm-link` is structured 
> to start with an empty module ("Composite"), then link in the first module 
> specified on the command line, then link in the rest of the modules 
> specified.  So we can't disambiguate between the initial empty module being 
> linked against the initial source file with the MDNode vs 2 full modules 
> where 1 is missing the MD node.  (ie. for two input source files, the main 
> module linking logic is run twice, not once).
> 
> See: https://reviews.llvm.org/D103851
> 
> ---
> 
> In that case, I just plan to remove this test case then, since it wasn't 
> being `RUN` anyways.
Ah ok, that makes sense. For regular LTO via the linker we also start with an 
empty module and link into it, so it would have the same issue.

Presumably we could detect and handle the case of linking into a completely 
empty module? But that can be revisited later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103048

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to