[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

This revision was automatically updated to reflect the committed changes. Closed by commit rG60ee88599098: [clang][asm goto][slh] Warn if asm goto + SLH (authored by zbrid). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid updated this revision to Diff 265282. zbrid added a comment. ClangFormat diagnostic definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743 Files:

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

jyu2 accepted this revision. jyu2 added a comment. This revision is now accepted and ready to land. LGTM. Please changed format for td file. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:247 + + def warn_slh_does_not_support_asm_goto : Warning< +

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid added a comment. In D79743#2044676 , @jyu2 wrote: > This looks good to me. Could you also add a test to use this new DiagGroup > (-Wno-slh-asm-goto)? > > Thanks. > > Jennifer Done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid updated this revision to Diff 265081. zbrid added a comment. Add test; Update command for existing test Also rename file to match warning flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743 Files:

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

jyu2 added a comment. This looks good to me. Could you also add a test to use this new DiagGroup (-Wno-slh-asm-goto)? Thanks. Jennifer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid updated this revision to Diff 264740. zbrid marked an inline comment as done. zbrid added a comment. Update to emit the warning in the parser Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743 Files:

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid added a comment. @jyu2 and @mattdr - updated to address your comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743 ___ cfe-commits mailing list

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid marked 3 inline comments as done. zbrid added a comment. > Do you mean runtime crash? If so, I think error should be emit, so that > programmer can remove use of "asm goto" and recompile. This would be a compile time crash. At some point the X86SpeculativeLoadHardening pass in the

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

jyu2 added a comment. I don’t know what consequences is of using asm goto under SLH. In general, if asm-goto is not allowed, the diagnostic should be emitted during the parser. If asm-goto is not allowed under specified condition, the diagnostic should be emitted during sema. Diagnostic

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

mattdr added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:248-249 + def warn_slh_does_not_support_gcc_asm_goto : Warning< +"speculative load hardening does not support use of GCC asm goto. asm goto " +"detected with SLH">,

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

craig.topper added a comment. In D79743#2035575 , @mattdr wrote: > Not sure we want to move this into the Parser -- SLH is a property of code > generation, and I think it's possible (through LTO?) to mismatch the flags > between parse and codegen.

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

mattdr added a comment. Not sure we want to move this into the Parser -- SLH is a property of code generation, and I think it's possible (through LTO?) to mismatch the flags between parse and codegen. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

jyu2 added a comment. In D79743#2034945 , @zbrid wrote: > In D79743#2034792 , @jyu2 wrote: > > > Two questions: > > 1> What happen under SLH, will asm goto gets removed, or a runtime problem? > > 2> Should we

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid added a comment. In D79743#2034792 , @jyu2 wrote: > Two questions: > 1> What happen under SLH, will asm goto gets removed, or a runtime problem? > 2> Should we emit error or warning in the Parser instead? 1. SLH crashes with an unhelpful

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid updated this revision to Diff 263819. zbrid added a comment. Update to fix failing test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/ https://reviews.llvm.org/D79743 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

jyu2 added a comment. Two questions: 1> What happen under SLH, will asm goto gets removed, or a runtime problem? 2> Should we emit error or warning in the Parser instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79743/new/

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

zbrid created this revision. zbrid added reviewers: mattdr, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Asm goto is not supported by SLH. Warn if an instance of asm goto is detected while SLH is enabled. Test included. Repository: rG LLVM Github Monorepo