[PATCH] D30671: [GlobalISel] Translate floating-point negation

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 7 09:17:56 PST 2017


Hi Volkan,

> On Mar 7, 2017, at 1:26 AM, Volkan Keles via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> volkan added inline comments.
> 
> 
> ================
> Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:174-185
> +bool IRTranslator::translateFSub(const User &U, MachineIRBuilder &MIRBuilder) {
> +  // -0.0 - X --> G_FNEG
> +  if (isa<Constant>(U.getOperand(0)) &&
> +      U.getOperand(0) == ConstantFP::getZeroValueForNegation(U.getType())) {
> +    MIRBuilder.buildInstr(TargetOpcode::G_FNEG)
> +        .addDef(getOrCreateVReg(U))
> +        .addUse(getOrCreateVReg(*U.getOperand(1)));
> ----------------
> kristof.beyls wrote:
>> Hi Volkan,
>> 
>> It's unclear to me why we really need a G_FNEG generic opcode.
>> If the idea is to later on be able to generate a target-specific FNEG instruction, why not have a pattern that can map G_FSUB 0.0, X to a targets specific FNEG, e.g.FNEGDr  for AArch64, during the InstructionSelect phase?
>> Or would the existing DAGISel patterns that contain fneg grow horribly complex if we don't have G_FNEG?
>> 
>> Unless there is a good reason for having an explicit G_FNEG opcode, it seems to me it's best to keep the number of generic opcodes as small as possible?
>> 
>> Thanks,
>> 
>> Kristof
> Hi Kristof,
> 
> Sorry for the lack of explanation. We need to have G_FNEG in order to lower G_FSUB in Legalizer as it may not be legal for some targets.

Just to clarify, you are saying that for instance G_FSUB float is not legal but G_FNEG float is. Thus the problem is that we don’t have a way to express that G_FSUB float 0.0, X is legal but G_FSUB non-0.0 is not.

Is that the rationale?

Thanks,
-Quentin 

> 
> Thanks,
> Volkan
> 
> 
> https://reviews.llvm.org/D30671
> 
> 
> 



More information about the llvm-commits mailing list