nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
This is still incorrect. The indices for the hi/low words are backwards. You
can easily demonstrate this with a test case such as:
a.c:
vector double test() { return (vector double)4.23422e12; }
b.c:
#include <stdio.h>
vector double test();
int main(int argc, const char **argv) {
vector double vec = test();
return printf("{ %g, %g }\n", vec[0], vec[1]);
}
$ clang -O3 a.c b.c -mcpu=pwr10
$ ./a.out
{ 5.55224e+224, 5.55224e+224 }
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9405
+ DAG.getNode(PPCISD::XXSPLTI32DX, dl, MVT::v2i64, SplatNode,
+ DAG.getTargetConstant(0, dl, MVT::i32),
+ DAG.getTargetConstant(Lo, dl, MVT::i32));
----------------
The high word goes to index zero and the low word goes to index 1. This is
backwards.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9366
+ SplatNode = DAG.getNode(
+ PPCISD::XXSPLTI32DX, SDLoc(SplatNode), MVT::v2i64, SplatNode,
+ DAG.getTargetConstant(isLE ? 1 : 0, SplatNode, MVT::i32),
----------------
Conanap wrote:
> amyk wrote:
> > I think I'm still a little confused by this. Do we not need `dl` when we do
> > `getNode()` here?
> Hey so the DL is supplied from `SplatNode`'s definition, and then we pass in
> `SDLoc(SplatNode)` here as the proper SDLoc after new instr def.
Why? Please don't use implicit conversion this way.
`DAG.getTargetConstant()` takes and `SDLoc`, you are passing an `SDValue`. This
of course works because the former has a constructor that does the implicit
conversion, but this is completely unnecessary and actually makes code less
readable.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90173/new/
https://reviews.llvm.org/D90173
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
