[PATCH] D138661: [AMDGPU][MC] Correct handling of mandatory literals

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:3387
+  if (!AMDGPU::isSISrcOperand(Desc, OpIdx) ||
+      AMDGPU::isKImmOperand(Desc, OpIdx)) {
     return false;
----------------
dp wrote:
> dp wrote:
> > Joe_Nash wrote:
> > > This looks a bit weird to me because isSISrcOperand used to be true for KImm, until I created the mandatory literal logic. I'm not sure what the desired semantics of isSISrcOperand are. Can you try making isSISrcOperand used to be true for KImm and seeing if there is any fallout?
> > `isSISrcOperand` does return true for `KImm` and seems it always did. I just added a limitation on `KImm` operands - they can no longer be interpreted as inline constants.
> > 
> > Before the mandatory literal logic was introduced, this limitation on `KImm` operands was unnecessary because only one literal was allowed for affected instructions.
> > 
> > Could you describe your concern in more detail?
> Are you suggesting to see if it is feasible to make `isSISrcOperand` return false for `KImm`?
> Are you suggesting to see if it is feasible to make `isSISrcOperand` return false for `KImm`?

Yes.

Sorry, I said the reverse of what I meant. 


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

https://reviews.llvm.org/D138661