llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.04k stars 11.98k forks source link

Suboptimal lowering of constant, negative, anyext function arguments or return values #38440

Open asb opened 6 years ago

asb commented 6 years ago
Bugzilla Link 39092
Version trunk
OS All
CC @ecnelises,@hfinkel

Extended Description

Consider a function returning a constant negative value, e.g.

define i16 @​neg_const() nounwind { ret i16 -2047 }

Compile this on a target for which i16 is not a legal type, e.g. PowerPC. For llc -mtriple=powerpc, the following assembly is generated: lis 3, 0 ori 3, 3, 65436 blr

We'd rather see: li 3, -100 blr

The issue is that SelectionDAGBuilder::visitRet will call SelectionDAG::getCopyToParts with ExtendKind = ISD::ANY_EXTEND. This calls SelectionDAG::getNode with ISD::ANY_EXTEND, which will recognise it has a constant argument and choose to zero-extend it. This generates the i32 constant 65436 (i.e. zext of int16(-100)) rather than the preferable -100.

By the time target instruction selection code is reached, there is no way to know that a sign-extended form of the original constant could have been used.

As a quick experiment I changed the constant-folding code in SelectionDAG::getNode so it performs a sign-extend for ANY_EXTEND input. That fixes this particular issue as expected but leads to incorrect codegen elsewhere, though I haven't yet had a chance to explore in more detail.

asb commented 5 years ago

They're not equal, but they're both valid lowerings because the return value is anyext.

llvmbot commented 5 years ago

In fact,

    lis 3, 0
ori 3, 3, 65436

is not equal

   li 3, -100

Becuae the r3 register is different, the first pattern the high bits of r3 is zero, but for the second, the higt bits of r3 is one.

asb commented 6 years ago

A target could fix things up in TgtISelDAGToDAG by retrieving the Function, checking the attributes on the return type, and modifying the constant return value if necessary.

However there's the same issue with constant, negative, anyext arguments:

declare void @​callee(i16)

define void @​caller() nounwind { call void @​callee(i16 -100) ret void }

In the general case, it's not really plausible to match a ConstantSDNode to the original argument in the Function given the splitting that may have taken place during legalisation.

Always sign-extending negative constant anyext arguments or return values would be preferable for targets like PowerPC, ARM and RISC-V.