[PATCH] D93983: RegAllocFast: Do not free later early-clobbered registers.

[Freddy, Ye via Phabricator via llvm-commits]

FreddyYe added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1241
+      // Do not free later early-clobbered registers when MI is MSVCInlineAsm
+      if (MI.isMSInlineAsm()) {
+        unsigned J = 0;
----------------
arsenm wrote:
> FreddyYe wrote:
> > arsenm wrote:
> > > I don't understand what's special about MSVC inline asm. Aren't the constraints semantically the same for any asm type?
> > MSVC inline asm is wrote in Intel assembly format. So the method isMsInlineAsm() here uses flag `inteldialect` to distinguish from GNU inline asm. (I don't know why this flag didn't show in this MIR test, but it indeed works.) 
> > 
> > From the investigation I've done so far, I have several conclusions:
> > 1. MSVC inline asm doesn't specify semantically `input`, `output`, `clobber` in source. But LLVM IR has such semantics. So does GNU inline asm.
> > 2. When clang front-end deals with MS inline asm, it has such rules to set `input`, `output`, `clobber`:
> >     1) always set eax as output 
> >     2) set the dest register in each asm as a clobber
> > The rules above will then generate corner case like this patch's IR and MIR test.
> > 
> > Yuanke mentioned about set early clobber in LLVM IR (i.e. =&r). I did a experiment. It indeed can pass this bug without this patch. But seems like this rule is for user side. Becasue I found that clang also cannot set `=&r` for GNU inline asm when output register is written to before the last asm. But clang can deal with similar semantics GNU inline case, because eax is not set as clobber by front-end. (https://gcc.godbolt.org/z/We3b9c).
> > 
> > 
> There shouldn't be a semantic difference. Ultimately the MIR should have the same flags/semantics regardless of the syntax used. This is either correct for all asm dialects, or it's generally incorrect
I agree with you that there shouldn't be semantic difference. Maybe the old implementation just happen to cover this issue luckily. I'd like to listen to more front-end's voice for this issue. Welcome to comment there! [[ https://reviews.llvm.org/D94466| D94466]]




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93983