[llvm] r298358 - [GlobalISel] Move isTriviallyDead to Utils. NFC.

[Ahmed Bougacha via llvm-commits]

On Tue, Mar 21, 2017 at 11:40 AM, Quentin Colombet via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Hi Volkan and Ahmed,
>
>> On Mar 21, 2017, at 3:47 AM, Volkan Keles via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: volkan
>> Date: Tue Mar 21 05:47:35 2017
>> New Revision: 298358
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=298358&view=rev
>> Log:
>> [GlobalISel] Move isTriviallyDead to Utils. NFC.
>>
>> Make it accessible by the targets to avoid code duplication.
>>
>> Modified:
>>    llvm/trunk/include/llvm/CodeGen/GlobalISel/Utils.h
>>    llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
>>    llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/Utils.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/Utils.h?rev=298358&r1=298357&r2=298358&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/GlobalISel/Utils.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/Utils.h Tue Mar 21 05:47:35 2017
>> @@ -45,6 +45,10 @@ unsigned constrainOperandRegClass(const
>>                                   MachineInstr &InsertPt, const MCInstrDesc &II,
>>                                   unsigned Reg, unsigned OpIdx);
>>
>> +/// Check whether an instruction \p MI is dead: it only defines dead virtual
>> +/// registers, and doesn't have other side effects.
>> +bool isTriviallyDead(const MachineInstr &MI, const MachineRegisterInfo &MRI);
>> +
>> /// Report an ISel error as a missed optimization remark to the LLVMContext's
>> /// diagnostic stream.  Set the FailedISel MachineFunction property.
>> void reportGISelFailure(MachineFunction &MF, const TargetPassConfig &TPC,
>>
>> Modified: llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp?rev=298358&r1=298357&r2=298358&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp Tue Mar 21 05:47:35 2017
>> @@ -48,29 +48,6 @@ void InstructionSelect::getAnalysisUsage
>>   MachineFunctionPass::getAnalysisUsage(AU);
>> }
>>
>> -/// Check whether an instruction \p MI is dead: it only defines dead virtual
>> -/// registers, and doesn't have other side effects.
>> -static bool isTriviallyDead(const MachineInstr &MI,
>> -                            const MachineRegisterInfo &MRI) {
>> -  // If we can move an instruction, we can remove it.  Otherwise, it has
>> -  // a side-effect of some sort.
>> -  bool SawStore = false;
>> -  if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore))
>> -    return false;
>> -
>> -  // Instructions without side-effects are dead iff they only define dead vregs.
>> -  for (auto &MO : MI.operands()) {
>> -    if (!MO.isReg() || !MO.isDef())
>> -      continue;
>> -
>> -    unsigned Reg = MO.getReg();
>> -    // Keep Debug uses live: we don't want to have an effect on debug info.
>> -    if (TargetRegisterInfo::isPhysicalRegister(Reg) || !MRI.use_empty(Reg))
>> -      return false;
>> -  }
>> -  return true;
>> -}
>> -
>> bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
>>   const MachineRegisterInfo &MRI = MF.getRegInfo();
>>
>>
>> Modified: llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp?rev=298358&r1=298357&r2=298358&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp Tue Mar 21 05:47:35 2017
>> @@ -47,6 +47,27 @@ unsigned llvm::constrainOperandRegClass(
>>   return Reg;
>> }
>>
>> +bool llvm::isTriviallyDead(const MachineInstr &MI,
>> +                           const MachineRegisterInfo &MRI) {
>> +  // If we can move an instruction, we can remove it.  Otherwise, it has
>> +  // a side-effect of some sort.
>> +  bool SawStore = false;
>> +  if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore))
>> +    return false;
>> +
>> +  // Instructions without side-effects are dead iff they only define dead vregs.
>> +  for (auto &MO : MI.operands()) {
>> +    if (!MO.isReg() || !MO.isDef())
>> +      continue;
>> +
>> +    unsigned Reg = MO.getReg();
>> +    // Keep Debug uses live: we don't want to have an effect on debug info.
>
> I’m actually wondering about that one.
> Debug information is not supposed to alter codegen.
>
> WDTY?

You're absolutely right, this is terribly wrong.  Fixed in r298460, by
erasing the instructions regardless and dropping the DBG_VALUEs.

Adrian explained to me how this is what both clang and swift (and
probably other users) expect, as FastISel has the same behavior.

clang has allocas everywhere, and swift uses a sideeffect inlineasm
with a "r" constraint, which means we'll currently fallback, but will
eventually do the right thing.

-Ahmed

> Cheers,
> -Quentin
>
>> +    if (TargetRegisterInfo::isPhysicalRegister(Reg) || !MRI.use_empty(Reg))
>> +      return false;
>> +  }
>> +  return true;
>> +}
>> +
>> void llvm::reportGISelFailure(MachineFunction &MF, const TargetPassConfig &TPC,
>>                               MachineOptimizationRemarkEmitter &MORE,
>>                               MachineOptimizationRemarkMissed &R) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits