[LLVMdev] Preserving the order of WRITE_REGISTER, READ_REGISTER in IR and ISelDAG

Hal Finkel hfinkel at anl.gov
Tue May 5 18:03:29 PDT 2015


Hi Nick,

We normally do code review on the llvm-commits list, moving the discussion there... (more comments inline)

----- Original Message -----
> From: "Nicholas Paul Johnson" <Nicholas.Paul.Johnson at DEShawResearch.com>
> To: llvmdev at cs.uiuc.edu
> Sent: Friday, May 1, 2015 11:00:42 AM
> Subject: Re: [LLVMdev] Preserving the order of WRITE_REGISTER, READ_REGISTER in IR and ISelDAG
> 
> FYI, I have submitted a bug and a proposed fix:
> https://llvm.org/bugs/show_bug.cgi?id=23389
> 
> >-----Original Message-----
> >From: llvmdev-bounces at cs.uiuc.edu
> >[mailto:llvmdev-bounces at cs.uiuc.edu]
> >On Behalf Of Johnson, Nicholas Paul
> >Sent: Wednesday, April 29, 2015 1:26 PM
> >To: llvmdev at cs.uiuc.edu
> >Subject: [LLVMdev] Preserving the order of WRITE_REGISTER,
> >READ_REGISTER
> >in IR and ISelDAG
> >
> >Hello,
> >
> >I have a few questions regarding intrinsics llvm.write_register,
> >llvm.read_register and the corresponding selection DAG node types
> >ISD::WRITE_REGISTER, ISD::READ_REGISTER.  As background, I'll
> >mention that
> >Clang emits these in response to variables declared as ``register
> >int eax
> >asm("eax");''
> >
> >(1) Per include/llvm/IR/Intrinsics.td, the intrinsic
> >llvm.write_register is
> >modeled with a memory side effect, but llvm.read_register is not
> >(i.e, the
> >intrinsic is declared with IntrNoMem).  If I understand correctly,
> >this means
> >that the relative ordering of calls to llvm.write_register will be
> >preserved,
> >but that optimizations may freely reorder llvm.read_register.  Why
> >is this?  It
> >seems like llvm.read_register should be ordered w.r.t.
> >llvm.write_register,
> >or the register-asm syntax is pretty much useless.

First, yes, we should not allow reordering of register reads with register writes, and as unfortunately we need to model this with memory dependencies because we don't have anything else.

It should not be modeled as read-write, however, but just as a read (thus, I think that IntrReadMem is appropriate).

> >
> >(2) Per SelectionDAGBuilder::visitIntrinsicCall, the Chain operand
> >for
> >ISD::WRITE_REGISTER SDNodes is derived from the second operand of
> >the
> >llvm.write_register intrinsic call (i.e., from the value being
> >written to the
> >registger), as follows:
> >
> >  // excerpt from lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> >  case Intrinsic::write_register: {
> >    Value *Reg = I.getArgOperand(0);
> >    Value *RegValue = I.getArgOperand(1);
> >    SDValue Chain = getValue(RegValue).getOperand(0);
> >    SDValue RegName =
> >      DAG.getMDNode(cast<MDNode>(cast<MetadataAsValue>(Reg)-
> >>getMetadata()));
> >    DAG.setRoot(DAG.getNode(ISD::WRITE_REGISTER, sdl, MVT::Other,
> >    Chain,
> >      RegName, getValue(RegValue)));
> >    return nullptr;
> >  }
> >
> >Is this a bug?  It seems that the chain should instead be the
> >selection DAG
> >root, as so:
> >
> >-    SDValue Chain = getValue(RegValue).getOperand(0);
> >+   SDValue Chain = DAG.getRoot();

No, I think the existing code is right. The SDAG intrinsic node should take the incoming chain as its first argument.

 -Hal

> >
> >
> >Thank you,
> >Nick
> >
> >
> >_______________________________________________
> >LLVM Developers mailing list
> >LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> >http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list