[PATCH] D64414: Do not set ReadNone attribute on intrinsics with side effects

[Momchil Velikov via Phabricator via llvm-commits]

chill added a comment.

In D64414#1575894 <https://reviews.llvm.org/D64414#1575894>, @lebedev.ri wrote:

> > If an intrinsic is defined without outputs, but having side effects, it still can be removed completely from the program
>
> This kinda sounds slightly backwards.
>  *If* there are side-effects, then i'd expect it not to be dropped regardless of `readnone` status.


Fair enough.

I had the issue that the `tcommit` (see child patch) was being discarded during instruction selection because

  static bool isFoldedOrDeadInstruction(const Instruction *I,
                                        FunctionLoweringInfo *FuncInfo) {
    return !I->mayWriteToMemory() && // Side-effecting instructions aren't folded.
           !I->isTerminator() &&     // Terminators aren't folded.
           !isa<DbgInfoIntrinsic>(I) &&  // Debug instructions aren't folded.
           !I->isEHPad() &&              // EH pad instructions aren't folded.
           !FuncInfo->isExportedInst(I); // Exported instrs must be computed.
  }

was true for the call insn.

Is the better fix adding a function attribute, e.g `def SideEffects : EnumAttr<"sideeffects">;` , etc and then changing the expression to be

    return !I->mayWriteToMemory() && // Side-effecting instructions aren't folded.
                 !I->hasSideEffects() && // Has unspecified side effects
                 !I->isTerminator() &&     // Terminators aren't folded.
  ....




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

https://reviews.llvm.org/D64414