Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR39092
Status NEW
Importance P enhancement
Reported by Alex Bradbury (asb@lowrisc.org)
Reported on 2018-09-26 13:49:01 -0700
Last modified on 2020-11-02 22:53:57 -0800
Version trunk
Hardware PC All
CC fwage73@gmail.com, hfinkel@anl.gov, llvm-bugs@lists.llvm.org, shkzhang@cn.ibm.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
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.
Quuxplusone commented 5 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.
Quuxplusone 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.
Quuxplusone commented 5 years ago

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