[PATCH] D51108: [PowerPC] Fix wrong ABI for i1 stack arguments for PPC32

[Lion Yang via Phabricator via llvm-commits]

LionNatsu added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:5471
+    // Ensure callee will get either 0x00000001 or 0x00000000.
+    if (Arg.getValueType() == MVT::i1)
+      Arg = DAG.getNode(ISD::ZERO_EXTEND, dl, MVT::i32, Arg);
----------------
hfinkel wrote:
> In your test case, the arguments have the zeroext attribute. That seems right. However, that being the case, should this be checking for i1, or should we check for Flags.isZExt()?
Good suggestion. I am working on this now.
Making an i1 argument "sigext" means it will pass either 0xFFFFFFFF or 0x00000000.

But now the last question is which one should be the default behaviour, ZERO_EXTEND or SIGN_EXTEND?

PPC32 ABI said:
> Values shorter than 32 bits are sign-extended or zero-extended, depending on whether they are signed or unsigned.
It is quite ambiguous for one-bit type because Spec did not mention how boolean should be implemented.

For comparison, PPC64 ELFv2 ABI also said:
> Map simple integer types (char, short, int, long, enum) to a single doubleword. Sign or zero extend values shorter than a doubleword to a doubleword based on whether the source data type is signed or unsigned.
But additionally:
> • A Boolean value is represented as a byte with a value of 0 or 1. If a byte with a value other than 0 or 1 is evaluated as a boolean value (for example, through the use of unions), the behavior is undefined.
So the byte of a *bool* should be either 0x01 or 0x00 hence signedness will be pointless on PPC64.

ISO C++ said:
In 6.7.1 Fundamental types
> Values of type bool are either true or false. [ Note: There are no signed, unsigned, short, or long bool types or values. — end note ] Values of type bool participate in integral promotions.
In 7.3.6 Integral promotions
> A prvalue of type bool can be converted to a prvalue of type int, with false becoming zero and true becoming one.

`std::is_signed` in C++ 11, returns `T(-1) < T(0)`. `-1` turned to `true` and `true` turned to `1` by integral promotions, `1 < 0` is false.

I think all these things suggest us to set zeroext as default. Any thought?



https://reviews.llvm.org/D51108