[PATCH, PING] Fix common-code ppcf128 component access on little-endian
Alp Toker
alp at nuanti.com
Mon Jun 30 13:46:45 PDT 2014
On 30/06/2014 23:01, Hal Finkel wrote:
> ----- Original Message -----
>> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
>> To: "llvm-commits" <llvm-commits at cs.uiuc.edu>
>> Sent: Monday, June 30, 2014 2:14:53 PM
>> Subject: [PATCH, PING] Fix common-code ppcf128 component access on little-endian
>>
>>
>>
>> Hello,
>>
>> the PowerPC 128-bit long double data type (ppcf128 in LLVM) is in
>> fact a
>> pair of two doubles, where one is considered the "high" or
>> more-significant
>> part, and the other is considered the "low" or less-significant part.
>> When
>> a ppcf128 value is stored in memory or a register pair, the high part
>> always comes first, i.e. at the lower memory address or in the
>> lower-numbered register, and the low part always comes second. This
>> is
>> true both on big-endian and little-endian PowerPC systems. (Similar
>> to how
>> with a complex number, the real part always comes first and the
>> imaginary
>> part second, no matter the byte order of the system.)
>>
>> This is currently implemented incorrectly for little-endian systems
>> in
>> LLVM. There are three issues I've noticed in my tests on ppc64le:
>>
>> - When printing an immediate ppcf128 constant to assembler output,
>> current
>> code in emitGlobalConstantFP does indeed correctly emit the high part
>> first
>> on big-endian systems, but emits the *low* part first on
>> little-endian
>> systems.
>> - When lowering a ppcf128 type to a pair of f64 types in SelectionDAG
>> (which is used e.g. when generating code to load an argument into a
>> register pair), low and high part are inverted on little-endian
>> systems.
>> - In a related issue, because lowering ppcf128 into a pair of f64
>> must
>> operate differently from lowering an int128 into a pair of i64,
>> bitcasts
>> between ppcf128 and int128 must not be optimized away by the DAG
>> combiner.
>>
>> The attached patch fixes these issues; this makes all native testing
>> on
>> powerpc64le pass (including full test-suite and bootstrap), when my
>> not-yet-published ELFv2 patches are also applied.
>>
>> I'd appreciate a review; is this OK to commit?
> The code itself looks fine, but with the exception of the printing code, is lacking regression tests. Please add regression tests to cover the other changes, and then commit.
Just a drive-by remark, but do you think there could be documentation
value in splitting the check out into a utility function like
hasBigEndianRepresentation(MVT::SimpleValueType T) instead of checking
MVT::ppcf128 specifically at each point of use?
Alp.
>
> Thanks again,
> Hal
>
>> (See attached file: diff-llvm-le-ppcf128)
>>
>>
>> Mit freundlichen Gruessen / Best Regards
>>
>> Ulrich Weigand
>>
>> --
>> Dr. Ulrich Weigand | Phone: +49-7031/16-3727
>> STSM, GNU/Linux compilers and toolchain
>> IBM Deutschland Research & Development GmbH
>> Vorsitzende des Aufsichtsrats: Martina Koederitz |
>> Geschäftsführung: Dirk
>> Wittkopp
>> Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
>> Stuttgart, HRB 243294
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
--
http://www.nuanti.com
the browser experts
More information about the llvm-commits
mailing list