[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

bmahjour accepted this revision. bmahjour added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128653/new/ https://reviews.llvm.org/D128653

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

bmahjour added inline comments. Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:33 +// CHECK-MASS-AFN: __xl_sin +// CHECK-NO-MASS-FAST-NOT: __xl_sin{{_finite}} +// CHECK-NO-MASS-AFN-NOT: __xl_sin{{_finite}} I don't think this works the way you expect it:

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

bmahjour added inline comments. Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:33-34 +// CHECK-MASS-AFN: __xl_sin +// CHECK-NOMASS-FAST-NOT: __xl_sin_finite +// CHECK-NOMASS-AFN-NOT: __xl_sin +// CHECK: blr Please also check for CHECK-NOMASS-FAST-NOT:

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

bmahjour added a comment. The description should explain why this change is necessary. Comment at: clang/test/CodeGen/lower-mass-end-to-end.c:1 +// RUN: %clang -mllvm -enable-ppc-gen-scalar-mass -O3 -fapprox-func --target=powerpc64le-unknown-linux-gnu -S %s -o -| FileCheck %s

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

bmahjour added inline comments. Comment at: lld/test/ELF/lto/discard-value-names.ll:4 ; RUN: ld.lld -shared -save-temps %t.o -o %t2.o ; RUN: llvm-dis < %t2.o.0.0.preopt.bc | FileCheck %s I see `--plugin-opt=opaque-pointers` is being explicitly specified for

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour accepted this revision. bmahjour added a comment. This revision is now accepted and ready to land. Apart from some minor inline comments this revision addresses all my outstanding comments. LGTM. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:17747 +

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:386 + if (TM.getOptLevel() == CodeGenOpt::Aggressive){ +setOperationAction(ISD::FSIN , MVT::f64, Custom); +setOperationAction(ISD::FCOS , MVT::f64, Custom); what

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

bmahjour added inline comments. Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:273-276 -; VF-TWO-CHECK-DAG: [[LOOPID_MV]] = distinct !{[[LOOPID_MV]], [[LOOPID_DISABLE_VECT:!.*]]} -; VF-TWO-CHECK-DAG: [[LOOPID_EV]] = distinct

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

bmahjour added inline comments. Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:273-276 -; VF-TWO-CHECK-DAG: [[LOOPID_MV]] = distinct !{[[LOOPID_MV]], [[LOOPID_DISABLE_VECT:!.*]]} -; VF-TWO-CHECK-DAG: [[LOOPID_EV]] = distinct

[PATCH] D114088: [PowerPC] Add BCD add/sub/cmp builtins

bmahjour added a comment. @nemanjai how come the changes in `altivec.h` and `clang/test/CodeGen/builtins-ppc-p8vector.c` have been removed in the latest diff and the commit above? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114088/new/

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

bmahjour added inline comments. Comment at: clang/lib/Headers/altivec.h:17349 +static __inline__ vector unsigned char __ATTRS_o_ai +vec_bperm(vector unsigned char __a, vector unsigned char __b) { + return __builtin_altivec_vbpermq(__a, __b); bmahjour wrote: >

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

bmahjour added inline comments. Comment at: clang/lib/Headers/altivec.h:17337 static __inline__ vector long long __ATTRS_o_ai vec_vbpermq(vector unsigned char __a, vector unsigned char __b) { return __builtin_altivec_vbpermq(__a, __b); lei wrote: > bmahjour

[PATCH] D106191: [clang] Option control afn flag

bmahjour added inline comments. Comment at: clang/include/clang/Driver/Options.td:1732-1733 NegFlag>; -def fapprox_func : Flag<["-"], "fapprox-func">, Group, Flags<[CC1Option, NoDriverOption]>, - MarshallingInfoFlag>, ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>; defm

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added a comment. In D101759#2971567 , @efriedma wrote: > errno handling for math library functions is a mess. Currently, we don't > model it properly; we just mark the calls "readnone" and hope for the best. > If you don't want to fix that,

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

bmahjour added inline comments. Comment at: clang/lib/Headers/altivec.h:17337 static __inline__ vector long long __ATTRS_o_ai vec_vbpermq(vector unsigned char __a, vector unsigned char __b) { return __builtin_altivec_vbpermq(__a, __b); This should be

[PATCH] D106409: [PowerPC] Add diagnostic for out of range values for vec_cts,vec_ctf

bmahjour added a comment. In D106409#2893592 , @nemanjai wrote: > My preference would be to just truncate the value with `& 0x1F`. It won't > produce any code in the binary and will work as expected. That's fine with me too. Repository: rG LLVM

[PATCH] D106409: [PowerPC] Add diagnostic for out of range values for vec_cts,vec_ctf

bmahjour added a comment. Thanks for this patch @ZarkoCA. We have six of these functions in altivec: `vec_ctd, vec_ctf, vec_cts, vec_ctsl, vec_ctu, vec_ctul`. They are all defined as macros in altivec.h and not all of them map to the builtins checked in this patch (eg `vec_ctul`). I realize

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added inline comments. Comment at: clang/include/clang/Driver/Options.td:1726 NegFlag>; -def fapprox_func : Flag<["-"], "fapprox-func">, Group, Flags<[CC1Option, NoDriverOption]>, - MarshallingInfoFlag>, ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>; +defm

[PATCH] D105666: [PowerPC] [Altivec] Use signed comparison for vec_all_* and vec_any_* interfaces that compare vector bool types with vector signed types

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2071ce9d4559: [Altivec] Use signed comparison for vec_all_* and vec_any_* interfaces (authored by bmahjour). Repository: rG LLVM Github Monorepo

[PATCH] D105666: [PowerPC] [Altivec] Use signed comparison for vec_all_* and vec_any_* interfaces that compare vector bool types with vector signed types

bmahjour created this revision. bmahjour added reviewers: nemanjai, cebowleratibm, PowerPC, rzurob. bmahjour added projects: PowerPC, LLVM. Herald added subscribers: shchenz, kbarton. bmahjour requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

bmahjour added a comment. In D103615#2852430 , @krasimir wrote: > I'm unfamiliar with the altivec API. What's a reasonable source code update > that preserves the current default behavior `-altivec-src-compat=mixed` under > `-altivec-src-compat=xl`,

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

bmahjour added a comment. > (generally: disabling the test in non-asserts builds isn't the right path, > modifying the test so it doesn't depend on asserts IR naming is the right > path) Agreed. > Yes, probably removing the entry: check would be sufficient - give it a test > locally and see

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

bmahjour added a comment. In D103615#2846245 , @dblaikie wrote: > As mentioned in the reverting commit - these tests fail in non-asserts > builds, because they assume named IR instructions (like the named entry BB > label), which aren't provided on a

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

bmahjour added a comment. In D103615#2847047 , @stefanp wrote: > I'm sorry I missed the asserts requirement. > I will recommit this patch after I add `REQUIRES: asserts`. Instead of disabling the tests for non-assert builds, can we just remove the

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

bmahjour added a comment. @jansvoboda11 This change is causing the following LIT tests to fail on AIX: Clang :: ClangScanDeps/headerwithdirname.cpp Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp The reason seems to be related to the fact that `-fno-integrated-as` is on by

[PATCH] D103615: [Clang] Add option to handle behaviour of vector bool/vector pixel.

bmahjour accepted this revision. bmahjour 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/D103615/new/ https://reviews.llvm.org/D103615

[PATCH] D102094: [AIX][PowerPC] Remove error when specifying mabi=vec-default on AIX

bmahjour added a comment. LGTM...I'll approve this change unless there are any objections by EOD tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102094/new/ https://reviews.llvm.org/D102094 ___ cfe-commits mailing list

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

bmahjour added a comment. Sorry I didn't mention this in my earlier comment about the option name, but I think that all inconsistencies in handling vector bool/pixel types should be controlled by a single compatibility option. For example the current special handling of initialization (splat

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

bmahjour added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:248 + enum class VectorCompareCompatKind { +// Default clang behaviour. +// All vector compares produce scalars except vector Pixel and vector bool. How about adding

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

bmahjour added a comment. As far as I can see, there is no good reason for the special treatment of vector bool/pixel going forward. Could we drop this special treatment, or at least change the default to use a scalar results across the board (consistent with XL's behaviour and clang's current

[PATCH] D101209: [PowerPC] Provide fastmath sqrt and div functions in altivec.h

bmahjour accepted this revision. bmahjour 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/D101209/new/ https://reviews.llvm.org/D101209

[PATCH] D101209: [PowerPC] Provide fastmath sqrt and div functions in altivec.h

bmahjour requested changes to this revision. bmahjour added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15130 + Value *Y = EmitScalarExpr(E->getArg(1)); + auto Ret = Builder.CreateFDiv(X, Y, "recipdiv"); +

[PATCH] D101130: [PowerPC] Provide XL-compatible builtins in altivec.h

bmahjour added a comment. LGTM Comment at: clang/test/CodeGen/builtins-ppc-xlcompat.c:6 +// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \ + // RUN: -triple powerpc64le-unknown-unknown -emit-llvm %s -o - \ +// RUN: -D__XL_COMPAT_ALTIVEC__ | FileCheck %s

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour added a comment. In D90159#2466599 , @MaskRay wrote: > Should this have some tests? Even if guarded by `REQUIRES:` if some feature > is needed. Test coverage for the DDG functionality has been added under LIT and unittests. I've opened

[PATCH] D78938: Make LLVM build in C++20 mode

bmahjour added inline comments. Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40 /// Static polymorphism: delegate implementation (via isEqualTo) to the /// derived class. + bool operator==(const DGEdge ) const { jfb wrote: > That comment, so

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour added a comment. In D90159#2454905 , @bmahjour wrote: > In D90159#2453805 , @Meinersbur > wrote: > >> Can I help fixing the Windows build problem? > > I think I have a fix (please see the updated patch),

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour updated this revision to Diff 311897. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90159/new/ https://reviews.llvm.org/D90159 Files: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp llvm/include/llvm/Analysis/CFGPrinter.h

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour added a comment. In D90159#2453805 , @Meinersbur wrote: > Can I help fixing the Windows build problem? I think I have a fix (please see the updated patch), but don't have access to a windows machine to verify. Would you be able to try building

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfd4a10732c8b: [DDG] Data Dependence Graph - DOT printer (authored by bmahjour). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour updated this revision to Diff 311678. bmahjour added a comment. fix formatting and use interleaveComma instead of interleave. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90159/new/ https://reviews.llvm.org/D90159 Files:

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour added inline comments. Comment at: llvm/include/llvm/Analysis/DDG.h:480 +return OS.str(); + unsigned count = 0; + for (auto : Deps) { Meinersbur wrote: > [suggestion] Instead of count, you can use `llvm::enumerate`. I think it's > nicer to check

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

bmahjour updated this revision to Diff 310607. bmahjour marked 5 inline comments as done. bmahjour added a comment. Herald added subscribers: cfe-commits, dexonsmith, ecnelises, martong, javed.absar, MatzeB. Herald added a project: clang. Thanks for the review @Meinersbur and sorry for taking so