Add TargetCustom type to PseudoSourceValue

[kariddi]

Summary

PseudoSourceValue can be used to attach a target specific value for "well behaved" side-effects lowered from target specific intrinsics.
This is useful whenever there is not an LLVM IR Value around when representing such "well behaved" side-effected operations in backends by attaching a MachineMemOperand with a custom PseudoSourceValue as this makes the scheduler not treating them as "GlobalMemoryObjects" which triggers a logic that makes the operation act like a barrier in the Schedule DAG.

This patch adds another Kind to the PseudoSourceValue object which is "TargetCustom". It indicates a type of PseudoSourceValue that has a target specific meaning (aka. LLVM shouldn't assume any specific usage for such a PSV)

[kariddi]

Updated MIRPrinter to handle this new enum val in switch case.

[kariddi]

Mmm, actually I think this will interact funnily with MIR as this is not a "generic" backend concept, but is very target specific.

Should something like this even being serializable/parsable to/from MIR?

[arsenm]

Could you extend this to accept any greater value than the max as TargetCustomN in case a target wants to distinguish multiple different kinds of values?

[arsenm]

if you want tests for this, I think AMDGPU could use this for texture operations which currently don't have set MMOs resulting in the scheduler problem.

[kariddi]

Do you mean change "isTargetCustom()" to something like (not final):

unsigned isTargetCustom() const {

unsigned KindVal = (unsigned) Kind;
return (KindVal >= TargetCustom) ? ((KindVal+1) - TargetCustom) : 0;

}

such that it returns 1, 2, 3 ... if the Kind is bigger or equal to TargetCustom and 0 if it is not a TargetCustom?

[arsenm]

In D12920#247564, @kariddi wrote:

Do you mean change "isTargetCustom()" to something like (not final):

unsigned isTargetCustom() const {

unsigned KindVal = (unsigned) Kind;
return (KindVal >= TargetCustom) ? ((KindVal+1) - TargetCustom) : 0;

}

such that it returns 1, 2, 3 ... if the Kind is bigger or equal to TargetCustom and 0 if it is not a TargetCustom?

Yes, exactly

[kariddi]

In D12920#247563, @arsenm wrote:

if you want tests for this, I think AMDGPU could use this for texture operations which currently don't have set MMOs resulting in the scheduler problem.

Yeah, that would be great.
This is the kind of use from where this idea comes from.

[kariddi]

• Updated to implement Matt's suggestion.

• Speculatively modified the MIR printer to not handle the TargetCustom PSV as it should be lower than the level MIR printer/parser operates .... but I'm not sure of that and I need a look from somebody working on MIR as I don't know a lot about it.

[qcolombet]

Hi Marcello,

Add Alex and or Duncan as reviewers. They are the ones that know about the MIR serialization.

Cheers,
-Quentin

[kariddi]

Done that, thanks Quentin!

Marcello

[arsenm]

include/llvm/CodeGen/PseudoSourceValue.h

67

Since this returns the Kind ID value instead of a bool, a name like getTargetCustom would be better

[kariddi]

include/llvm/CodeGen/PseudoSourceValue.h

67

Agreed , done in the next patch

[kariddi]

Still waiting for comments about the MIR serialization stuff.

[pete]

I might be mis-remembering, but I thought we were trying to remove PSV’s?

Only commit I could see which remotely suggested that might be the case is r206255 where the comment contained

"Anything that needs to use either a PseudoSourceValue* and Value* is strongly encouraged to use a MachinePointerInfo instead.”

I’ve CCed Nick to see if he can remember whether there was more work to come on PSV’s or whether they are here to stay. If they are going to be removed in future then its something to think about here before adding more code which relies on them.

Pete

[kariddi]

Thanks Pete for your comment!
I think the commit message you mentioned means that , because Value and PseudoSourceValue are not anymore in the same class heirarchy, trying to use them in a way that requires both an LLVM Value and a PseudoSourceValue at the same time makes it "impossible
Basically I think that the word "either" in the commit message should actually have been "both".

That said if the same can be achieved without passing a PseudoSourceValue to the MachinePointerInfo that would be great even if probably it would provide a little bit less flexibility in the capability of specifying target specific inforrmation used by later passes as the MachinePointerInfo alone doesn't have any field that can be used to store custom data. (Maybe the data can be somehow packed in the offset, but doesn't seem very flexible and is a little bit hacky).

I tried removing the attachment of the PseudoSourceValue to the MachineMemOperand and actually the scheduler continues to not treat the node as a scheduling barrier as if the PSV was attached , so from that point of view it is fine using a MachinePointerInfo without a value attached. It still makes the ScheduleDAG builder happy!

I'm interested though in hearing what is the intended destiny of PSVs (or their intended usage).
Thanks for CCing whoever might know more about it!

[kariddi]

Resubmtting with full context

include/llvm/CodeGen/PseudoSourceValue.h

     ConstantPool,
     FixedStack,
     GlobalValueCallEntry,
-    ExternalSymbolCallEntry
+    ExternalSymbolCallEntry,
+    TargetCustom
   };
 
 private:

   bool isGOT() const { return Kind == GOT; }
   bool isConstantPool() const { return Kind == ConstantPool; }
   bool isJumpTable() const { return Kind == JumpTable; }
+  unsigned getTargetCustom() const {
+    return (Kind >= TargetCustom) ? ((Kind+1) - TargetCustom) : 0;
+  }
 
   /// Test whether the memory pointed to by this PseudoSourceValue has a
   /// constant value.

lib/CodeGen/MIRPrinter.cpp

       printLLVMNameWithoutPrefix(
           OS, cast<ExternalSymbolPseudoSourceValue>(PVal)->getSymbol());
       break;
+    case PseudoSourceValue::TargetCustom:
+      llvm_unreachable("TargetCustom pseudo source values are not supported");
+      break;
     }
   }
   printOffset(Op.getOffset());