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

[Volkan Keles via llvm-commits]

Hi Quentin, 

> On Mar 7, 2017, at 9:17 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> 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?

Yes, that’s the rationale. In this way, we will be able to lower G_FSUB if G_FNEG is legal.

Volkan

> 
> Thanks,
> -Quentin 
> 
>> 
>> Thanks,
>> Volkan
>> 
>> 
>> https://reviews.llvm.org/D30671 <https://reviews.llvm.org/D30671>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170307/b23fe886/attachment-0001.html>