Calls to functions with signext/zeroext return values

Project Infrastructure LLVM Dev List Archives
1

In SelectionDAGBuilder::visitRet(), there is this bit of code:

        // FIXME: C calling convention requires the return type to be promoted
        // to at least 32-bit. But this is not necessary for non-C calling
        // conventions. The frontend should mark functions whose return values
        // require promoting with signext or zeroext attributes.
        if (ExtendKind != ISD::ANY_EXTEND && VT.isInteger()) {
          EVT MinVT = TLI.getRegisterType(*DAG.getContext(), MVT::i32);
          if (VT.bitsLT(MinVT))
            VT = MinVT;
        }

There have been a few discussions about this snippet on llvmdev in the
past[1][2][3], and there seems to be a general consensus that the
responsibility for promoting to the 'int' type should be transfered to
the front end and the signext/zeroext attributes eliminated. But
that's not what I'm interested in discussing here.

What I'd like to ask about is calls to functions that have a
signext/zeroext attribute on their return value. As far as I can tell,
there isn't any corresponding promotion of the return value to i32 in
visitCall(). Should there be?

I ran into problems in a DSP back end that I'm working on where the
return conventions for i16 and i32 are slightly different (they are
both returned in the same accumulator register, but at different
offsets within the accumulator). The callee promoted the return value
to i32, but the caller was expecting it to be returned as an i16.

So I made some changes to SelectionDAGBuilder (see attached patch) to
truncate return values back to their declared sizes and that seemed to
fix the problems for my DSP backend. But these changes broke some
regression tests in other back ends. Specifically,

    LLVM :: CodeGen/MSP430/2009-11-05-8BitLibcalls.ll
    LLVM :: CodeGen/MSP430/indirectbr.ll
    LLVM :: CodeGen/X86/h-registers-3.ll

The failures in the MSP430 tests are particularly troubling because
they are assertion failures in LegalizeDAG because i32 is not a legal
type.

The X86 failure seems less serious. Based on my limited knowledge of
X86 assembly, it looks like the back end is generating code that works
but that is different from what the test expects.

So my questions:

1. Should visitCall() promote the return value to i32 as is done in visitRet()?
2. If so, any suggestions on how to fix my patch or the MSP430 back
end so it won't crash the MSP430 tests?
3. If not, what options do I have for handling the convention mismatch
in my back end? The only one that I can see is ensuring that i32 and
i16 return values use compatible conventions.

-Ken

[1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-February/012840.html
[2] http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-May/014449.html
[3] http://lists.cs.uiuc.edu/pipermail/llvmdev/2009-February/020078.html

call-extended-return.patch (2.62 KB)

2

Promoting the return value is unsafe for bool returns on x86-64, which in the latest revision of the ABI only guarantees that the top 7 bits of the 8-bit register are 0.

Cameron

3

My comment is a bit off, because the question of what type to make the return value is somewhat orthogonal to the question of which zext assert we should add.

Cameron

4

I'm not sure I follow. Won't a zeroext attribute on a bool return
value ensure that it will be zero-extended to 32 bits by the callee?
Or does the X86 backend consider such functions unlowerable (via
TargetLowering::CanLowerReturn()) and thereby bypass the extension to
32 bits in SelectionDAGBuilder::visitRet() making a promotion in the
caller unnecessary?

-Ken

5

The X86 backend currently zero-extends them to 32 bits, but according to the ABI it need only zero-extend them to 8 bits. I'm going to change this in a few minutes to expose some additional optimization opportunities.

Cameron

6

I ran into problems in a DSP back end that I'm working on where the
return conventions for i16 and i32 are slightly different (they are
both returned in the same accumulator register, but at different
offsets within the accumulator). The callee promoted the return value
to i32, but the caller was expecting it to be returned as an i16.

So I made some changes to SelectionDAGBuilder (see attached patch) to
truncate return values back to their declared sizes and that seemed to
fix the problems for my DSP backend.

That patch has been rendered obsolete by some recent changes by
Cameron. Attached is an updated one.

So my questions:

...
2. If so, any suggestions on how to fix my patch or the MSP430 back
end so it won't crash the MSP430 tests?

With Cameron's addition of a getTypeForExtendedInteger() hook in
TargetLowering, I've been able to work around the failures in the
MSP430 back end by overriding the extension size to i16 (see the
attached patch). This seems like it would be the appropriate size for
the architecture -- since the current default of i32 isn't a legal
type -- but I really have no idea what affects it has on the runtime
library or whether it conforms to an official ABI (does one exist?).
Anton, do you have any comments?

-Ken

call-extended-return.r2.patch (2.59 KB)

msp430-i16-extend.patch (1.21 KB)