[PATCH] D159520: [BOLT][AArch64] Fix instrumentation deadloop

Wed Sep 20 10:57:15 PDT 2023

Amir added inline comments.

================
Comment at: bolt/include/bolt/Core/MCPlusBuilder.h:624

+  virtual bool isExclusive(const MCInst &Inst) const {
+    llvm_unreachable("not implemented");
----------------
yota9 wrote:
> Amir wrote:
> > "Exclusive" is ARM-specific term, so it would help to either clarify it a bit here, or come up with a generic term – maybe "isLoadLockStoreConditionalInst"?
> I agree. But we have a bunch of arch-specific interfaces, e.g. is adrp, islea, isRISCVCall. Usually I'm not insisting on such questions, but to my taste "isLoadLockStoreConditionalInst"  is a little bit too complicated :)) I like the current variant to be honest, but maybe isAArch64Exclusive is good-enough compromise and in the future add isArch prefix for arch-specific things? 
Yes, `isAArch64Exclusive` is a good compromise. 

================
Comment at: bolt/lib/Passes/Instrumentation.cpp:315-316

+  if (hasExclusiveMemop(Function))
+    return;
+
----------------

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159520