[llvm-dev] Change prototype for TargetInstrInfo::foldMemoryOperandImpl

[Prathamesh Kulkarni via llvm-dev]

Hi,
While working on https://reviews.llvm.org/D79785, we wanted to define
foldMemoryOperandImpl hook for Thumb1, that folds load, indirect call
to direct call  tLDRpci, tBLX -> tBL. This triggered an assertion
error with expensive checks turned on in MachineVerifier because the
newly created tBL insn by
Thumb1InstrInfo::foldMemoryOperandImpl had memory operands of LoadMI
attached by TargetInstrInfo::foldMemoryOperand, which is done
unconditionally:

// Copy the memoperands from the load to the folded instruction.
if (MI.memoperands_empty()) {
  NewMI->setMemRefs(MF, LoadMI.memoperands())

In this case, we don't want the memory loads to be added to MI from
LoadMI. Should there be some mechanism for target specific
foldMemoryOperandImpl hook to signal to foldMemoryOperand to not add
memory operands from LoadMI ?

I was wondering if either of these approaches look good ?
(a) Make foldMemoryOperandImpl return two values via std::pair or use
OUT param (reference). The first is the MI instruction, and second a
bool whether or not to add memory operands from LoadMI. This will
however require adjusting other targets that override
foldMemoryOperandImpl. Would that be OK ?

(b) Define another method in TargetInstrInfo that decides whether or
not to add memory operands to the MI returned by
foldMemoryOperandImpl, which will not require to modify existing code,
but not sure if it's a good idea ?

(c) Make foldMemoryOperandImpl itself add memory operands, which will
require somewhat more complicated changes in targets that already
implement foldMemoryOperandImpl.

I would be grateful for suggestions on how to proceed.

Thanks,
Prathamesh