[PATCH] D75941: [x86][seses] No LFENCEs in basic blocks w/o loads

George Burgess IV via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 10 17:33:25 PDT 2020


george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM modulo a few cosmetic nits. Similar to the previous review, please wait for approval from someone with more ownership here before landing.

Thanks!



================
Comment at: llvm/lib/Target/X86/X86SpeculativeExecutionSideEffectSuppression.cpp:55
+    OmitLFENCEInBasicBlocksWithoutLoads("x86-seses-omit-lfence-in-bb-without-loads",
+                      cl::desc("Omit LFENCE in basic blocks without any loads even if there are stores."),
+                      cl::init(false), cl::Hidden);
----------------
nit: please wrap at 80cols, potentially by "splitting the" " string"


================
Comment at: llvm/lib/Target/X86/X86SpeculativeExecutionSideEffectSuppression.cpp:90
+    if (OmitLFENCEInBasicBlocksWithoutLoads) {
+
+      bool FoundLoad = false;
----------------
nit: please delete this line


================
Comment at: llvm/test/CodeGen/X86/speculative-execution-side-effect-suppression-omit-lfence-in-bb-without-loads.ll:12
+; CHECK-FLAGGED: .globl _Z4buzzv                # -- Begin function _Z4buzzv
+; CHECK-FLAGGED: .p2align 4, 0x90
+; CHECK-FLAGGED: .type _Z4buzzv, at function
----------------
If these tests are checking for consecutive lines, consider using `CHECK-FLAGGED-NEXT`. Otherwise, FileCheck won't complain if there are intervening instructions (e.g., an incorrectly placed lfence)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75941





More information about the llvm-commits mailing list