This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Validate dst and src2 non-overlapping restriction in asm
ClosedPublic

Authored by rampitec on Jan 24 2022, 4:27 PM.

Diff Detail

Event Timeline

rampitec created this revision.Jan 24 2022, 4:27 PM
rampitec requested review of this revision.Jan 24 2022, 4:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 4:27 PM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm added inline comments.Jan 24 2022, 4:38 PM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

Can't you get the width directly from the operand register class without making an assumption about the number of register units?

3646–3653

This is a weird overlap check. Can't you just check if you encounter the register in MCRegAliasIterator

rampitec updated this revision to Diff 402715.Jan 24 2022, 4:45 PM
rampitec marked an inline comment as done.

There is already a helper function isRegIntersect using MCRegAliasIterator.

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

I cannot get RC itself.

arsenm added inline comments.Jan 24 2022, 4:46 PM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

It's in the MCInstrDesc which you have

rampitec added inline comments.Jan 24 2022, 4:49 PM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

It is not:

struct MCRegisterDesc {
  uint32_t Name;      // Printable name for the reg (for debugging)
  uint32_t SubRegs;   // Sub-register set, described above
  uint32_t SuperRegs; // Super-register set, described above
 
  // Offset into MCRI::SubRegIndices of a list of sub-register indices for each
  // sub-register in SubRegs.
  uint32_t SubRegIndices;
 
  // RegUnits - Points to the list of register units. The low 4 bits holds the
  // Scale, the high bits hold an offset into DiffLists. See MCRegUnitIterator.
  uint32_t RegUnits;
 
  /// Index into list with lane mask sequences. The sequence contains a lanemask
  /// for every register unit.
  uint16_t RegUnitLaneMasks;
};

I guess because is a register can belong to different classes.

arsenm added inline comments.Jan 24 2022, 4:52 PM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

MCRI.getRegClass(Desc.OpInfo[OpIdx].RegClass)

rampitec updated this revision to Diff 402717.Jan 24 2022, 5:00 PM
rampitec marked 3 inline comments as done.
rampitec added inline comments.
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
3643

Ugh, InstrDesc, not RegDesc!

arsenm accepted this revision.Jan 26 2022, 2:42 PM
This revision is now accepted and ready to land.Jan 26 2022, 2:42 PM
This revision was landed with ongoing or failed builds.Jan 26 2022, 3:14 PM
This revision was automatically updated to reflect the committed changes.