zbrid marked 2 inline comments as done.
zbrid added a comment.
Thanks for the ping, Scott. I'll update this so I can get it submitted soon.
================
Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:200
+ if (!Args.hasArg(options::OPT_mno_lvi_cfi)) {
+ Features.push_back("+lvi-cfi");
+ LVIOpt = options::OPT_mlvi_cfi;
----------------
sconstab wrote:
> Would it be better to add `FeatureLVIControlFlowIntegrity` as a dependency
> for `FeatureSpeculativeExecutionSideEffectSuppression` in
> `llvm/lib/Target/X86/X86.td`?
Thanks for the tip! Yeah, I will update to do that, but it looks like that only
ensures an error will be thrown if they aren't used together rather than
ensuring one is enabled when the other is enabled. Am I misunderstanding?
================
Comment at:
llvm/lib/Target/X86/X86SpeculativeExecutionSideEffectSuppression.cpp:90
+ const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>();
+ if (!Subtarget.useSpeculativeExecutionSideEffectSuppression() &&
+ !EnableSpeculativeExecutionSideEffectSuppression)
----------------
sconstab wrote:
> Is it really necessary to have the target feature and the CLI flag? If SESES
> is required for, say, a *.ll file, then `+seses` can always be added as a
> target feature.
I think there should be a way to turn on SESES without lvi-cfi. Similar to how
there are flags to turn on SLH in various configurations. I'll see if I can
lower the number of flags while still enabling that possibility.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79910/new/
https://reviews.llvm.org/D79910
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
