ychen marked an inline comment as done.
ychen added inline comments.

================
Comment at: llvm/test/CodeGen/X86/jmc-instrument.ll:2
+; Check that the flag symbol is not full-qualified.
+; RUN: llc < %s -enable-jmc-instrument | FileCheck %s
+
----------------
hans wrote:
> ychen wrote:
> > hans wrote:
> > > hans wrote:
> > > > Since it's an IR pass, I think we don't need to have an llc test at all.
> > > I still think it's unusual to have an llc test for an IR pass. Are there 
> > > any other examples where we do something similar?
> > yes, it is pretty rare. I wanted to test the `CodeViewDebug.cpp` change, 
> > however, the code path only triggers when the JMC pass is run. Using `llc` 
> > came to mind. I'm open to any alternative.
> Oh, I see. But does the test check the right thing, then? I only see CHECKs 
> for instructions, not debug info.
> 
> And isn't it enough to use IR with a global variable in the .msvcjmc section 
> as input? I think the tighter the test, the better.
It turns out that the `CodeViewDebug.cpp` change is not needed anymore. Delete 
this test too. Thanks for help spotting this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118428

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to