steven.zhang added a comment.
Some code style comments.
Comment at: clang/include/clang/Driver/Options.td:2603
def mno_spe : Flag<["-"], "mno-spe">, Group;
+def mefpu2 : Flag<["-"], "mefpu2">, Group;
def mabi_EQ_vec_extabi : Flag<["-"], "mabi=vec-extabi">, Group,
steven.zhang added inline comments.
Comment at: clang/lib/AST/ItaniumMangle.cpp:2668
// ::= g # __float128
+ // ::= g # __ibm128
// UNSUPPORTED:::= Dd # IEEE 754r decimal floating point (64 bits)
This is a bit
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM as it seems the missing part. Please also fix the error message if with
Power8.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM as I grep the whole repo with this patch applied, no QPX any more.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92329/new/
steven.zhang added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:418
+ // should prefer D-form if LXVX / STXVX uses a ZERO or ZERO8
+ if (MI.getOpcode() == PPC::LXVX || MI.getOpcode() == PPC::STXVX) {
+LLVM_DEBUG(dbgs()
steven.zhang added a comment.
LGTM now and thank you for the double check. But please hold on for some days
in case someone has concern on enabling the new macros(we are enabling the
__LONG_DOUBLE_IBM128__ by default now).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90208/new/
steven.zhang added a comment.
Sorry, accept the revision by mistake. Please hold on until my comments
addressed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90208/new/
https://reviews.llvm.org/D90208
steven.zhang accepted this revision.
steven.zhang added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/Basic/Targets/PPC.cpp:122
if (LongDoubleWidth == 128) {
Builder.defineMacro("__LONG_DOUBLE_128__");
steven.zhang added a comment.
I think some follow up is needed to optimize the code sequence for
cr = vec_test_div
if (rotate_and_mask(cr, 62))
...
For now, we will copy the cr to gpr, and shift it to 61-64 bit,then, extract
the bit and then compare to 0. the shift is not needed.
steven.zhang accepted this revision.
steven.zhang added a comment.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88278/new/
https://reviews.llvm.org/D88278
___
cfe-commits mailing list
steven.zhang added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2584
+// Vector test software functions.
+def : Pat<(i32 (int_ppc_vsx_xvtdivdp v2f64:$A, v2f64:$B)),
Vector test for software divide and sqrt
Repository:
rG LLVM Github
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82725/new/
https://reviews.llvm.org/D82725
steven.zhang added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10054
+
+ case Intrinsic::ppc_altivec_mtvsrbm: {
+// The llvm.ppc.altivec.mtvsrbm intrinsic can correspond to two different
Can we handle this inside the .td ? i.e.
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82726/new/
https://reviews.llvm.org/D82726
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM, thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85874/new/
https://reviews.llvm.org/D85874
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM. But please hold on for one more days to see if there is other comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
steven.zhang added inline comments.
Comment at: clang/lib/Basic/Targets/PPC.h:86
+
+HasStrictFP = true;
}
nemanjai wrote:
> I don't think we need this for now. Close is not quite there. @steven.zhang I
> would prefer that we initially turn this off and
steven.zhang added inline comments.
Comment at: llvm/test/MC/Disassembler/PowerPC/vsx.txt:2
# RUN: llvm-mc --disassemble %s -triple powerpc64-unknown-linux-gnu -mcpu=pwr7
| FileCheck %s
+# RUN: llvm-mc --disassemble %s -triple powerpc64-unknown-linux-gnu \
+# RUN:
steven.zhang added a comment.
This is a huge patch. I suggest you to split them into small ones for each kind
of builtins, so that, it would be more easy to review.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81836/new/
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM now and thank you for doing this. Please hold on for 2-3 days in case
others have comments. And thank you for pointing a potential issue of the
folding of fneg + fma. We will
steven.zhang added a comment.
LGTM overall with some minor comments.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14238
BuiltinID == PPC::BI__builtin_vsx_xvrspi)
- ID = Intrinsic::round;
+ ID = Builder.getIsFPConstrained() ?
steven.zhang added inline comments.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14069
+ BuiltinID == PPC::BI__builtin_vsx_xvrspim)
+ID = Intrinsic::floor;
+ else if (BuiltinID == PPC::BI__builtin_vsx_xvrdpi ||
Can we do it like this to
steven.zhang accepted this revision.
steven.zhang added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81355/new/
https://reviews.llvm.org/D81355
steven.zhang added a comment.
Herald added a subscriber: wuzish.
Shouldn't this be the last patch to commit after the backend supporting this
feature ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81355/new/
https://reviews.llvm.org/D81355
steven.zhang added a comment.
It LGTM now except one comment on the test. And it seems that, we still have
many other builtins implementation that didn't use the _Generic.
Comment at: clang/test/CodeGen/ppc-emmintrin.c:1
-// NOTE: Assertions have been autogenerated by
steven.zhang added a comment.
The PowerPC backend is also adding the constraint float point support and
almost done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80952/new/
https://reviews.llvm.org/D80952
steven.zhang added inline comments.
Comment at: clang/lib/Headers/altivec.h:13670
+ )
+#elif defined(__VSX__)
+#define vec_splats(N) \
vddvss wrote:
> steven.zhang wrote:
> > I am not sure if this is by intention. It is not
steven.zhang added inline comments.
Comment at: clang/lib/Headers/altivec.h:13670
+ )
+#elif defined(__VSX__)
+#define vec_splats(N) \
I am not sure if this is by intention. It is not semantics the same with this
change.
steven.zhang added inline comments.
Comment at: clang/lib/Basic/Targets/PPC.cpp:272
.Default(false);
Features["qpx"] = (CPU == "a2q");
Do we miss to define this macro ?
```
__POWER10_VECTOR__
```
Comment at:
steven.zhang added inline comments.
Comment at: clang/lib/Basic/Targets/PPC.cpp:272
.Default(false);
Features["qpx"] = (CPU == "a2q");
steven.zhang wrote:
> Do we miss to define this macro ?
> ```
> __POWER10_VECTOR__
> ```
steven.zhang added a comment.
I will commit the patch for you.
Repository:
rC Clang
https://reviews.llvm.org/D52074
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
31 matches
Mail list logo
