[PATCH] D40554: [PowerPC] Fix bugs in sign-/zero-extension elimination
Stefan Pintilie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 17 16:59:22 PST 2022
stefanp added inline comments.
================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:5092
// This function returns true if the machine instruction
// always outputs a value by sign-extending a 32 bit value,
----------------
amyk wrote:
> Question: should we update this to say if the `register` and not `machine instruction` since it is changed to `Reg` for the parameter?
> Or should it stay as machine instruction since you're checking for the different `MachineInstr` opcodes in this function?
> The same question I have applies to `definedByZeroExtendingOp()`.
I think it should probably say "machine instruction that defines this register" because that is what we are really interested in. We want to make sure that this register is already sign extended by the instruction that defines it. I'm going to re-write the comment to make it clear.
================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:5314
+ auto SrcExt = isSignOrZeroExtended(SrcReg, BinOpDepth, MRI);
+ return std::pair<bool, bool>(SrcExt.first | IsSExt, SrcExt.second | IsZExt);
}
----------------
amyk wrote:
> If the change to logical or still applies, we should probably update these accordingly.
Sure. I can change it to logical.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40554/new/
https://reviews.llvm.org/D40554
More information about the llvm-commits
mailing list