tejohnson added a comment. In D102742#2774639 <https://reviews.llvm.org/D102742#2774639>, @nickdesaulniers wrote:
> In D102742#2774562 <https://reviews.llvm.org/D102742#2774562>, @tejohnson > wrote: > >> In D102742#2774185 <https://reviews.llvm.org/D102742#2774185>, >> @nickdesaulniers wrote: >> >>> - upgrade module merge strategy to Error >> >> Probably want a test using llvm-link or llvm-lto to check this behavior >> (that alike flags are getting propagated as expected and that conflicting >> ones error) > > Not many tests under llvm/test/LTO or llvm/test/ThinLTO use `llvm-link`; is > there a more appropriate subdir for such a test for conflicting module > attributes? Hmm, maybe in llvm/test/Linker? Or you could presumably use llvm-lto. ================ Comment at: clang/include/clang/Driver/Options.td:3429 HelpText<"Use the given reg for addressing the stack-protector guard">, - MarshallingInfoString<CodeGenOpts<"StackProtectorGuardReg">, [{"none"}]>; + MarshallingInfoString<CodeGenOpts<"StackProtectorGuardReg">>; def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at function entry (x86/SystemZ only)">, ---------------- nickdesaulniers wrote: > tejohnson wrote: > > What's the effect of or reason for this change? > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 are > strings (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`). > It was inconsistent that one could be unspecified, where as the other could > be unspecified or `"none"` (but those 2 values were equivalent). > > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to check > that `StackProtectorGuardReg != "none"` rather than > `!StackProtectorGuardReg.empty()` below. > > I can change it back, but I think the asymmetry between > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in D88631, > and I missed that in code review. > > I don't think there would be any other observers of such a change. I see. Does unspecified mean something like just "-mstack-protector-guard-reg=" with nothing after the =? I didn't realize that was supported. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102742/new/ https://reviews.llvm.org/D102742 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
