[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added a comment. In D152914#4653669 , @lei wrote: > HI @nemanjai, Did you get a chance to post this as a github PR? Long overdue, but I finally have it up at https://github.com/llvm/llvm-project/pull/68919 Repository: rG LLVM Github

[PATCH] D116016: [Clang] [PowerPC] Emit module flag for current float abi

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

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added a comment. In D152914#4602914 , @ilinpv wrote: > Friendly ping, are there any questions remained to proceed with > target-independent __builtin_cpu_supports ? Sorry, I have not gotten back to this review in quite some time as I have been

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM. This is a good idea and we should go ahead with this for anyone that uses `vec_promote`, but it might be a good idea to improve codegen for the insert which might be more common.

[PATCH] D158065: [PowerPC] Implement builtin for mffsl

nemanjai accepted this revision. nemanjai added a comment. Thanks for implementing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158065/new/ https://reviews.llvm.org/D158065 ___ cfe-commits

[PATCH] D158066: [PowerPC] Fix use of FPSCR builtins in smmintrin.h

nemanjai added a comment. In D158066#4593961 , @qiucf wrote: > CC @amyk @quinnp Any comments about the naming? > > I see some `__builtin_ppc_xxx` are aliased into `__builtin_xxx` by > `defineXLCompatMacros`. But these are not XL-compatible builtins, and

[PATCH] D156351: clang driver throws error for -mabi=elfv2 or elfv2

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. Ah, ok. This makes sense. When we don't know anything about the ABI, it makes sense to rely on `-mabi`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D156076: [PowerPC][Clang] Remove constraint for initial-exec TLS mode on AIX

nemanjai added a comment. So we don't have code generation for this, but we are enabling it in the front end? What happens if we try to produce code for the IR this produces? It would make more sense to me to implement what is needed in the back end prior to allowing the front end to produce

[PATCH] D156076: [PowerPC][Clang] Remove constraint for initial-exec TLS mode on AIX

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. Please provide a description justifying this change. There is no context here for the viewer to determine whether this change makes sense. Repository: rG LLVM Github Monorepo

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:903-907 +// Load of a value provided by the system library at a fixed address. Used for +// accessing things like HWCAP word provided by GLIBC. +def int_fixed_addr_ld +:

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:903-907 +// Load of a value provided by the system library at a fixed address. Used for +// accessing things like HWCAP word provided by GLIBC. +def int_fixed_addr_ld +:

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added a comment. @arsenm Has my description adequately addressed your question? Do you think we should move forward with [some version of] this patch or do you have any fundamental objections? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added a comment. In D152914#4428692 , @ilinpv wrote: > Thank you for the patch, it comes in the right time - we are also working on > AArch64 __builtin_cpu_supports, and I was thinking how to make it more > general. > I uploaded our RFC

[PATCH] D155111: [clangd] Fix build failures observed on build bots for missing libs

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. Lets get this committed to unblock the bots and if it is not the correct/desired fix, the author can subsequently provide the more appropriate fix. Repository: rG LLVM Github Monorepo

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

nemanjai accepted this revision. nemanjai added a comment. No concerns from the perspective of PowerPC here. Of course, my focus is primarily on the server side of things but I am not aware of any other group that could be adversely affected. CHANGES SINCE LAST ACTION

[PATCH] D106409: [PowerPC] Truncate exponent parameter for vec_cts,vec_ctf

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

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:903-907 +// Load of a value provided by the system library at a fixed address. Used for +// accessing things like HWCAP word provided by GLIBC. +def int_fixed_addr_ld +:

[PATCH] D148697: [clang-tidy] Add more checks for functions which should be noexcept

nemanjai added a comment. In D148697#4422642 , @PiotrZSL wrote: > In D148697#4422629 , @nemanjai > wrote: > >> It was also causing PPC bots to break. I fixed it in >>

[PATCH] D148697: [clang-tidy] Add more checks for functions which should be noexcept

nemanjai added a comment. In D148697#4421739 , @Dinistro wrote: > In D148697#4421643 , @PiotrZSL > wrote: > >> @Dinistro It compiles & links with clang 16 also (shared libs). So I'm >> unable to reproduce. > >

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1785 + if (static_cast(TM).hasGlibcHWCAPAccess()) +OutStreamer->emitSymbolValue( This probably deserves a comment along the lines of: ``` // Emit a reference to a symbol

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

nemanjai created this revision. nemanjai added reviewers: PowerPC, RKSimon, pengfei, arsenm, t.p.northover. Herald added subscribers: steven.zhang, kbarton, hiraditya. Herald added a project: All. nemanjai requested review of this revision. Herald added subscribers: jdoerfert, wdng. Herald added

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

nemanjai accepted this revision. nemanjai added subscribers: maryammo, kamaub, stefanp. nemanjai added a comment. This revision is now accepted and ready to land. This looks fine to me. I'd like some of the devs that have added builtins with Sema checking in the past to have a look here as well.

[PATCH] D135171: FreeBSD: enable __float128 on x86 and powerpc64le

nemanjai accepted this revision. nemanjai added a comment. My comments are minor nits that don't require another review, so LGTM. Comment at: clang/lib/Basic/Targets/OSTargets.h:245-250 switch (Triple.getArch()) { -default: +case llvm::Triple::ppc64le: case

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

nemanjai added a comment. Actually, while you're here, can you please remove all calls to `SemaFeatureCheck()` that test builtins which are only available on a specific CPU? The Sema checking happens too early before the target info is populated, which does not allow for a very common idiom

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

nemanjai added a comment. In D143467#4195474 , @qiucf wrote: > - For builtins with matching instruction, use the required ISA/vector version > - For the rest builtins used in altivec.h, use requirements specified by the > header > - Keep the feature

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

nemanjai added a comment. Another note regarding the motivating test case: Use of `vector double` should require VSX. We don't really seem to have the ability to turn this off early enough to catch this though. It would seem that in the front end, the target features depend on the compilation

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

nemanjai added inline comments. Comment at: clang/include/clang/Basic/BuiltinsPPC.def:491 +TARGET_BUILTIN(__builtin_altivec_vabsduh, "V8UsV8UsV8Us", "", "altivec") +TARGET_BUILTIN(__builtin_altivec_vabsduw, "V4UiV4UiV4Ui", "", "altivec") qiucf wrote: > shchenz

[PATCH] D145141: [Driver] Reject -march= for ppc

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. I am perfectly fine with this. That option is quite problematic. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145141/new/

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG59cd692454c9: [PowerPC] Recognize long CPU name for -mtune in Clang (authored by nemanjai). Changed prior to commit:

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

nemanjai added a comment. In D144967#4165140 , @nemanjai wrote: > I'll provide an updated patch in a few min... Ah, what I mean is I will update the patch and push in a few min. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

nemanjai added a comment. I'll provide an updated patch in a few min... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144967/new/ https://reviews.llvm.org/D144967 ___ cfe-commits mailing list

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

nemanjai created this revision. nemanjai added reviewers: PowerPC, nathanchance, MaskRay. Herald added subscribers: steven.zhang, shchenz, kbarton. Herald added a project: All. nemanjai requested review of this revision. Herald added a project: clang. There are two ways of specifying a CPU on

[PATCH] D144293: [PowerPC] Fix the implicit casting for the emulated intrinsics

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM as long as the naming nit is addressed. Comment at: clang/lib/Headers/ppc_wrappers/emmintrin.h:57 typedef __vector unsigned char __v16qu; +typedef __vector float

[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

nemanjai accepted this revision. nemanjai added a comment. Thanks for doing this. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144321/new/ https://reviews.llvm.org/D144321 ___ cfe-commits mailing

[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

nemanjai added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:430-432 + if ((Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() || +Triple.getOSMajorVersion() >= 13)) || + Triple.isOSOpenBSD() || Triple.isMusl())

[PATCH] D142222: [PowerPC] Remove the lax warning for explicit casts

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM other than the missing template test. Comment at: clang/test/Parser/lax-conv.cpp:4 +// RUN: %clang_cc1 -triple=powerpc64-ibm-aix -target-feature +altivec

[PATCH] D140080: [clang][PPC] Supporting -mcpu=405

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. I don't think the comments require another round of review. Thanks for fixing this. Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:23-29 +static constexpr

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM. Thanks so much for fixing up the existing code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137511/new/

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. I am going to block this since the uses of the macros in `clang/lib/Headers/ppc_wrappers` will likely cause build bot failures. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

nemanjai added a comment. As mentioned by @q66 above, this can't go in until usage of this macro in the Clang/LLVM codebase is fixed. Looks like the uses in `clang/lib/Headers/ppc_wrappers` and some of the uses in `libcxx` and `libunwind` must be fixed while others should probably also be

[PATCH] D134371: [clang-doc] Add typedef/using information.

nemanjai added a comment. This causes failures with `-Werror` such as: https://lab.llvm.org/buildbot/#/builders/57/builds/22322 Please fix or revert. Comment at: clang-tools-extra/clang-doc/Representation.h:45 + IT_enum, + IT_typedef }; There are at least

[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

nemanjai added a comment. I am not crazy about adding the Boolean parameter here or about the name. Seems somewhat unclear when a caller wants to pass `true` there. What I think would be a more robust solution would be to use the same logic that decides whether to coerce the struct argument to

[PATCH] D85599: [PowerPC] Remove isTerminator for TRAP instruction

nemanjai added a comment. Herald added a project: All. I think we should handle this similarly to `SITargetLowering::splitKillBlock()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85599/new/ https://reviews.llvm.org/D85599

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

nemanjai added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10702 + +// If SafeStack or !StackProtector, kill_canary is not supported. +if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) || Again, this comment is

[PATCH] D131346: [clang] LLVM_FALLTHROUGH => [[fallthrough]]. NFC

nemanjai added a comment. Why? There are many years of precedent for using `LLVM_FALLTHROUGH` and it is very clear and obvious. What do we gain by getting rid of it? Don't get me wrong, I am not super opposed to using a standard string instead of an LLVM-specific macro. However, it seems that

[PATCH] D130526: [Driver][PowerPC] Support -mtune=

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. Thanks. We'll eventually start doing something with the option in the back end. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129855: [clang][PowerPC] Set lld as clang's default linker for PowerPC Linux

nemanjai added a comment. In D129855#3667191 , @MaskRay wrote: > In D129855#3662457 , @quinnp wrote: > >> In D129855#3657006 , @MaskRay >> wrote: >> >>> This is not

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. Please run clang-format, rebase and re-upload. It doesn't apply cleanly to ToT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128652: [PowerPC] Finished kill_canary implementation and debugging

nemanjai added a comment. "Made a new phabricator review because of git issues" is not an appropriate description of a review/revision. Hopefully the description you add will describe what this intrinsic is supposed to do. It seems to me that this is a poorly designed feature if it is meant to

[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX

nemanjai added a comment. I am ok with this change overall, I just have a couple of questions about naming of the option. 1. Is there any precedent for options that start with `-maix` or `-m` for any other OS? 2. Is `quadword` the best word to use? There is no type information and this is

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat.c:8 // RUN: -D__XL_COMPAT_ALTIVEC__ -target-cpu pwr8 | FileCheck %s -// RUN: %clang_cc1

[PATCH] D126540: PowerPC] Emit warning for incompatible vector types that are currently diagnosed with -fno-lax-vector-conversions

nemanjai added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7571-7573 + "Current bitcast for incompatible vector types (%0 and %1) are deprecated. " + "The default behaviour will change to what is implied by the " +

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

nemanjai added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15587 + +bool isUnaligned = (BuiltinID == PPC::BI__builtin_altivec_vinsw || +BuiltinID == PPC::BI__builtin_altivec_vinsd) This is strange. You don't need the

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

nemanjai added a comment. In D127310#3567472 , @MaskRay wrote: > Do you have more authoritative answer when /root/usr is used and when it > isn't? These suffixes were always part of the code and the mentioned changeset removed it without any

[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

nemanjai added subscribers: quinnp, nemanjai. nemanjai added a comment. The original toolchain detection added `root/usr` to the paths which was certainly necessary on PowerPC. Since that suffix is removed in this patch, our Redhat buildbot is now broken

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

nemanjai added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2152 ChosenToolsetVersion = ToolsetVersion; -ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str(); +ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str() + "/root/usr"; }

[PATCH] D126302: [PowerPC] Diagnose invalid combination with Altivec, VSX and soft-float

nemanjai created this revision. nemanjai added reviewers: Laurentiu, PowerPC. Herald added subscribers: shchenz, kbarton. Herald added a project: All. nemanjai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The current behaviour with

[PATCH] D125203: [PowerPC] Fix PPCISD::STBRX selection issue on A2

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. In D125203#3502651 , @tingwang wrote: > In D125203#3502433 , @nemanjai > wrote: > >> Why not also fix

[PATCH] D124060: [PowerPC] Enable CR bits support for Power8 and above.

nemanjai added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:3566 +Control the CR-bit tracking feature on PowerPC. ``-mcrbits`` (the enablement of CR-bit tracking support) is the default for POWER8 and above. + ``` ... is the default

[PATCH] D124060: [PowerPC] Enable CR bits support for Power8 and above.

nemanjai added inline comments. Comment at: clang/lib/Basic/Targets/PPC.cpp:519 .Default(false); + Features["crbits"] = llvm::StringSwitch(CPU) +.Case("ppc64le", true) shchenz wrote: > amyk wrote:

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

nemanjai added a comment. In D122983#3465122 , @aaron.ballman wrote: > In D122983#3465099 , @nemanjai > wrote: > >> @daltenty Can you please run this with the same config as the bot on one of >> our AIX

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

nemanjai added a comment. @daltenty Can you please run this with the same config as the bot on one of our AIX machines but be sure to pass `-i` if it's `make` or `-k 0` if it's `ninja`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983/new/

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. Also, please run `clang-format` on the changes. Comment at: clang/lib/Headers/altivec.h:19051 #ifdef __LITTLE_ENDIAN__ - return

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

nemanjai added a comment. In D122983#3463569 , @xbolva00 wrote: > But your link shows failures in compiler-rt/, not in llvm test-suite It shows both. But the `compiler-rt` ones have subsequently been fixed while the `test-suite` ones are still failing

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

nemanjai added a comment. This is still causing failures when building `test-suite`: https://lab.llvm.org/buildbot/#/builders/105/builds/24292 Why don't we just turn off this warning for the entire `test-suite` since I don't expect we want to modify the tests (as some of them have licenses that

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

nemanjai added a comment. In D67678#3446526 , @rsmith wrote: > @dexonsmith @arphaman What do we need to do to get this re-landed? FWIW, on the PPC side, we are working on cleaning up `altivec.h` and plan to eventually make `-fno-lax-vector-conversions`

[PATCH] D121637: [PowerPC] Fix EmitPPCBuiltinExpr to emit arguments once

nemanjai accepted this revision. nemanjai added a comment. Please add a test case that would cause a failure prior to this patch due to the argument being emitted more than once (i.e. the test case that prompted this patch). If that is already added and I just missed it, please add a note to

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nemanjai added a comment. In D120305#3346880 , @MaskRay wrote: > While I feel sorry for leaving clang-ppc64le-rhel red for some time and am > willing to fix issues if I have access to a ppc64 machine (especially > compiler-rt ones that I care about), >

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nemanjai added a comment. In D120305#3346144 , @MaskRay wrote: > In D120305#3345978 , @MaskRay wrote: > >> In D120305#3345810 , @RKSimon >> wrote: >> >>> @MaskRay The

[PATCH] D118753: [PowerPC] Fix __builtin_pdepd and __builtin_pextd to be 64-bit and P10 only.

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

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct member is passed to a function

nemanjai added a comment. Would it make sense (and would it be possible) to check the linkage of the callee? Presumably calling something like `static void localfunc(int *)` with an over-aligned member shouldn't be a problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

nemanjai added a comment. In D116085#3272200 , @nemanjai wrote: > This is causing buildbot failures with `-Werror` (i.e. > https://lab.llvm.org/buildbot/#/builders/57/builds/14322/steps/5/logs/stdio). > Please fix or revert. The failure is: > > >

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

nemanjai added a comment. This is causing buildbot failures with `-Werror` (i.e. https://lab.llvm.org/buildbot/#/builders/57/builds/14322/steps/5/logs/stdio). Please fix or revert. The failure is: .../llvm-project/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:204:38: error:

[PATCH] D118211: Add missing namespace to PPCLinux.cpp

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

[PATCH] D118110: [CMake] [Clang] Add CMake build option to specify long double format on PowerPC

nemanjai added inline comments. Comment at: clang/CMakeLists.txt:240 +set(ENABLE_PPC_IEEELONGDOUBLE OFF CACHE BOOL +"Enable IEEE binary128 as default long double format on PowerPC.") Do we need any error checking here? What happens if someone erroneously

[PATCH] D118110: [CMake] [Clang] Add CMake build option to specify long double format on PowerPC

nemanjai added a comment. It is probably not worth the effort since there won't be that many test cases that test the front end's IR generation for `long double`, but there should be a way to set up lit to know the default through its configuration files. Repository: rG LLVM Github Monorepo

[PATCH] D112906: [PowerPC] Emit warning for ieeelongdouble on older GNU toolchain

nemanjai added inline comments. Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:50 + if (!SupportIEEEFloat128(D, Triple, Args)) { +if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) { + StringRef ABIName = A->getValue(); Seems that this will be

[PATCH] D117181: [PowerPC] Use IEEE long double in proper toolchain

nemanjai added a comment. In D117181#3261396 , @jsji wrote: > So, the proposal is we change the default on Linux, so we will not change > the default based on the gcc version, it is based on OS and clang version > only. > but will emit warning if we

[PATCH] D117181: [PowerPC] Use IEEE long double in proper toolchain

nemanjai added a comment. I don't think this is the right approach. Personally, I would find it surprising if simply having GCC 12.1 on the system changes my `long double` default. Imagine if I simply install and use such a toolchain on a machine for whatever reason. If the default for such a

[PATCH] D112073: [PowerPC] Emit warning when SP is clobbered by asm

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

[PATCH] D106120: [PowerPC] Implement vector bool/pixel initialization under -faltivec-src-compat=xl

nemanjai added a comment. In D106120#3197209 , @q66 wrote: > Well, I'm more concerned about the actual semantics - as I see it, when using > generic vectors, they should be unaffected by the comparison behavior of > AltiVec vectors, which is not the

[PATCH] D116015: [PowerPC] Add generic fnmsub intrinsic

nemanjai added a comment. Converting more generic code to target-specific intrinsics is sometimes necessary to ensure the generic IR doesn't get transformed in a way that is disadvantageous. I believe that the description of this review claims that to be the case for these negated FMA's. The

[PATCH] D116016: [Clang] [PowerPC] Emit module flag for current float abi

nemanjai added a comment. We should not be emitting the attribute in modules that do not have any use of `long double`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116016/new/ https://reviews.llvm.org/D116016

[PATCH] D106120: [PowerPC] Implement vector bool/pixel initialization under -faltivec-src-compat=xl

nemanjai added a comment. In D106120#3196010 , @q66 wrote: > On x86_64, the program predictably prints `Dv4_i`. The semantics here are not new, but the warning is emitted for generic vector types as a result of https://reviews.llvm.org/D103615. This is

[PATCH] D114497: [PowerPC] Drop stdlib paths in freestanding tests

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. Seems fine to me but maybe give @MaskRay a couple of days to see if this adequately addresses his comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114540: Big-endian version of vpermxor

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM. Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2506 + def : Pat<(v16i8 (int_ppc_altivec_crypto_vpermxor_be v16i8:$a, +

[PATCH] D114540: Big-endian version of vpermxor

nemanjai added a comment. Please also add a test for this builtin to the front end test `clang/test/CodeGen/builtins-ppc-crypto.c` Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2499 (COPY_TO_REGCLASS $c, VSRC>; + def :

[PATCH] D113642: [PowerPC] Provide XL-compatible vec_round implementation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7bf937bbee3: [PowerPC] Provide XL-compatible vec_round implementation (authored by nemanjai). Repository: rG LLVM Github Monorepo CHANGES SINCE

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGc933c2eb3346: [PowerPC] Add BCD add/sub/cmp builtins (authored by nemanjai). Changed prior to commit: https://reviews.llvm.org/D114088?vs=387933=389238#toc Repository: rG LLVM Github Monorepo

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

nemanjai added inline comments. Comment at: clang/lib/Headers/altivec.h:19050 +} + +static __inline__ long __bcdcmpeq(vector unsigned char __a, NeHuang wrote: > Do we need to add a case for "__CR6_SO_REV"? It is defined in line 25 but not > used. I added it for

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

nemanjai created this revision. nemanjai added reviewers: bmahjour, PowerPC. Herald added subscribers: shchenz, kbarton, hiraditya. nemanjai requested review of this revision. Herald added projects: clang, LLVM. Support for builtins that use `bcdadd./bcdsub.` to add/subtract Binary Coded Decimal

[PATCH] D113306: [PowerPC] Allow MMA built-ins to accept non-void pointers and arrays

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

[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. Please provide a description for this patch which includes justification for why we want to allow conversion between the two types. I am of the impression that allowing the two

[PATCH] D111434: [PowerPC] PPC backend optimization on conditional trap intrustions

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM. There are some very minor nits that can be addressed on the commit. Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1018 +unsigned Opcode2 =

[PATCH] D113642: [PowerPC] Provide XL-compatible vec_round implementation

nemanjai created this revision. nemanjai added reviewers: PowerPC, rzurob, qiongsiwu. Herald added subscribers: shchenz, kbarton, hiraditya. nemanjai requested review of this revision. Herald added projects: clang, LLVM. The XL implementation of `vec_round` for `vector double` uses

[PATCH] D112285: [PowerPC] PPC backend optimization to lower int_ppc_tdw/int_ppc_tw intrinsics to TDI/TWI machine instructions

nemanjai accepted this revision. nemanjai added a comment. This revision is now accepted and ready to land. LGTM other than a number of stylistic changes. Feel free to address those on the commit. You also might want to give @amyk a bit of time to ensure her comments were adequately addressed.

[PATCH] D106409: [PowerPC] Truncate exponent parameter for vec_cts,vec_ctf

nemanjai requested changes to this revision. nemanjai added a comment. This revision now requires changes to proceed. I believe you are planning an update for this patch. Requesting changes to take it off the queue until you have uploaded the updated version. Repository: rG LLVM Github

[PATCH] D112285: [PowerPC] PPC backend optimization to lower int_ppc_tdw/int_ppc_tw intrinsics to TDI/TWI machine instructions

nemanjai requested changes to this revision. nemanjai added inline comments. This revision now requires changes to proceed. Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5001 + case ISD::INTRINSIC_VOID: { +if (N->getConstantOperandVal(1) == Intrinsic::ppc_tdw || +

[PATCH] D111434: [PowerPC] PPC backend optimization on conditional trap intrustions

nemanjai added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1011 + case PPC::TDI: + case PPC::TWI: { +MachineInstr *LiMIA = getVRegDefOrNull((1), MRI); Seems that we should be able to handle all 4 in the same block: -

  1   2   3   4   >