[PATCH, PING] Fix common-code ppcf128 component access on little-endian

Ulrich Weigand Ulrich.Weigand at de.ibm.com
Wed Jul 2 12:45:56 PDT 2014


Hal Finkel <hfinkel at anl.gov> wrote on 30.06.2014 22:55:06:

> > > > 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?
> >
> > Some of the code is ppcf128-specific anyway, but for the other cases,
> > I do like the suggestion. I don't like the name, however ;) -- how
> > about hasBigEndianMemberLayout?
>
> Or maybe hasBigEndianPartOrdering? [Simple value types don't really
> have members].

Here's an updated version of the patch that uses a helper routine
hasBigEndianPartOrdering.  It also adds tests to cover the other
changes.

In doing so I noticed that I missed a case in ExpandRes_BITCAST,
and once I added that, it turns out that the original change to
getCopyToParts is not necessary. (This is because that function
first uses a BITCAST to convert to an integer type, and then
operates on the integer type.  The endian swap for ppc_fp128
takes place during the BITCAST already; no further swap is
required.)

Does this look OK?

(See attached file: diff-llvm-le-ppcf128-new)

Thanks for the review!

Bye,
Ulrich

-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-llvm-le-ppcf128-new
Type: application/octet-stream
Size: 10930 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140702/3df1ce54/attachment.obj>


More information about the llvm-commits mailing list