[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

jcranmer-intel added a comment. So while trying to review this patch, I've discovered there's an annoying incompatibility between C and C++ here, in that C and C++ specify different rules on how to choose between `_Float64`, `double`, and `long double` if all are `binary64` (C says `_Float64`

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

This revision was automatically updated to reflect the committed changes. jcranmer-intel marked an inline comment as done. Closed by commit rGbf4923710333: [Clang] Enable -print-pipeline-passes in clang. (authored by jcranmer-intel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel marked 5 inline comments as done. jcranmer-intel added inline comments. Comment at: clang/test/CodeGen/print-pipeline-passes.c:8 +// CHECK: always-inline +// CHECK-SAME: verify +void Foo(void) {} aeubanks wrote: > aeubanks wrote: > > I believe

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel updated this revision to Diff 556574. jcranmer-intel added a comment. Replace verify with annotation-remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127221/new/ https://reviews.llvm.org/D127221 Files:

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel updated this revision to Diff 556483. jcranmer-intel added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127221/new/ https://reviews.llvm.org/D127221 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102 +outs() << "\n"; +return; + } aeubanks wrote: > I wouldn't return here, doesn't seem right that we'll skip running the opt > pipeline but continue with

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel updated this revision to Diff 556476. jcranmer-intel added a comment. Rebase to trunk. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127221/new/ https://reviews.llvm.org/D127221 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

jcranmer-intel added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319 +if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) || +TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half)) PromoteType = Context.DoubleTy;

[PATCH] D156506: [clang][Interp] Check floating results for NaNs

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:503 + // If during the evaluation of an expression, the result is not + // mathematically defined [...], the behavior is undefined. + // FIXME: C++ rules require us to not conform to IEEE

[PATCH] D156506: [clang][Interp] Check floating results for NaNs

jcranmer-intel added a comment. You definitely don't want these rules to apply to all qNaNs. It's when an input operand is an sNaN for many operations. Note that the result of an operation with an sNaN as input (and FP result type) is a qNaN output, and the only times that you get an sNaN

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1,

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1,

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added a comment. In D155546#4510691 , @aaron.ballman wrote: > It's not yet clear to me what happens when any of these functions encounter a > signaling NaN at compile time. CC @hubert.reinterpretcast @jcranmer-intel > @rsmith

[PATCH] D155394: [clang][Interp] Implement __builtin_fpclassify

jcranmer-intel added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:121-130 + char classify_nan [__builtin_fpclassify(+1, -1, -1, -1, -1, __builtin_nan(""))]; + char classify_snan[__builtin_fpclassify(+1, -1, -1, -1, -1,

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbcad161db3e6: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V. (authored by jcranmer-intel). Changed prior to commit:

[PATCH] D145150: clang: Emit nofpclass(nan inf) for -ffinite-math-only

jcranmer-intel added a comment. I'm generally okay with the approach of this patch. I'm not sufficiently well-versed in the clang codegen side of things to know if this covers all of the bases, and I'd appreciate someone who is familiar with that side of things to approve this patch. CHANGES

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

jcranmer-intel added a comment. I'm studiously ignoring the Clang and LLVM codegen changes here, but otherwise, I think the direction of this change is generally good. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1377-1378 llvm_unreachable("unknown denormal mode");

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added a comment. Friendly review ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141008/new/ https://reviews.llvm.org/D141008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D145150: clang: Emit nofpclass(nan inf) for -ffinite-math-only

jcranmer-intel added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2212 +static llvm::FPClassTest getNoFPClassTestMask(const LangOptions ) { + // TODO: Handle -fno-signaling-nans + llvm::FPClassTest Mask = llvm::fcNone; Clang doesn't have support

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986 + // for more details). + SmallVector IntParams = {0, 0, 0, 0, 0, 0}; + Anastasia wrote: > I think the list initialization doesn't do what you are trying to achieve >

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel updated this revision to Diff 501594. jcranmer-intel added a comment. Documentation tweaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141008/new/ https://reviews.llvm.org/D141008 Files: clang/include/clang-c/Index.h

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added a comment. Friendly review ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141008/new/ https://reviews.llvm.org/D141008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added inline comments. Comment at: clang/test/CodeGenOpenCL/opencl_types.cl:2 // RUN: %clang_cc1 -no-opaque-pointers -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-SPIR // RUN: %clang_cc1

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel updated this revision to Diff 498453. jcranmer-intel marked 5 inline comments as done. jcranmer-intel added a comment. Mostly test fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141008/new/ https://reviews.llvm.org/D141008

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

jcranmer-intel added a comment. In D142907#4132430 , @arsenm wrote: > I was thinking of changing the default in general to dynamic. I was going to > at least change the strictfp default in a follow up I had the same thought too, but I reflected a

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

jcranmer-intel added a comment. Not entirely sure where the best place to effect this (I think somewhere in the clang driver code?), but on further reflection, it feels like strict fp-model in clang should set the denormal mode to dynamic. CHANGES SINCE LAST ACTION

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel marked an inline comment as done. jcranmer-intel added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2260 -return CGF.CGM.getNullPointer(cast(ConvertType(DestTy)), - DestTy); +// The type may be a target

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel updated this revision to Diff 495866. jcranmer-intel marked 2 inline comments as done. jcranmer-intel added a comment. Herald added a subscriber: jdoerfert. This updates code, and rebases tests on top of trunk. Note: test issues still haven't been fixed, will fix that likely in

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

jcranmer-intel added a comment. Looking at the attribute logic here, there is conceptual room for both a `dynamic` and an `unknown` mode (i.e., you get a top and a bottom value), but I don't think there is value in distinguishing between them, so I'm fine with keeping just a `dynamic`. I

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

jcranmer-intel added a comment. From some of the verifier checks and tests, it looks like `target("x86.amx")` would also require some new type properties, to express its unsuitability for alloca-and-friends, as well as non-intrinsic arguments. The spelling change needs to be release noted, and

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel updated this revision to Diff 487960. jcranmer-intel added a comment. Fix some of the code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141008/new/ https://reviews.llvm.org/D141008 Files:

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added a comment. Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:80 + + if (useSPIRVTargetExtType(CGM)) { +switch (cast(T)->getKind()) { Anastasia wrote: > Perhaps it's best to split into separate functions and reflect in naming what >

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added inline comments. Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:40 +static llvm::Type *getSPIRVType(llvm::LLVMContext , StringRef BaseType, +StringRef OpenCLName, StringRef ReadSuffix) { + SmallVector IntParams = {0, 0, 0,

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel added a comment. Most of the testing for this change has been in conjunction with the changes in the SPIRV-LLVM-Translator repository here: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1799. I haven't updated the in-tree experimental target to support the

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

jcranmer-intel created this revision. Herald added subscribers: ldrumm, wenlei, ThomasRaoux, arphaman, Anastasia, yaxunl. Herald added a project: All. jcranmer-intel requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

jcranmer-intel added a comment. I don't normally handle name mangling, so I can't comment too much here, but I will note that Itanium ABI is planning on using DF16b for `std::bfloat16_t`: https://github.com/itanium-cxx-abi/cxx-abi/pull/147 Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Opcodes.td:502 + let Types = [AluTypeClass]; + let Args = [ArgFltSemantics]; + let HasGroup = 1; tbaeder wrote: > sepavloff wrote: > > tbaeder wrote: > > > jcranmer-intel wrote: > > > >

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:487 + if (S.inConstantContext()) +return true; + Not sure I understand the conditions that cause `S.inConstantContext()` to be true, which gives me some cause for concern.

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

jcranmer-intel added inline comments. Comment at: clang/test/Sema/constant-builtins-ilogb.cpp:53-63 +// assert smallest subnormal and largest finite numbers +static_assert(__builtin_ilogbf(1.40129846E-45f) == -149); +static_assert(__builtin_ilogbf(3.40282347E+38f) == 127); +

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

jcranmer-intel added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; Izaron wrote: > aaron.ballman wrote: > > jcranmer-intel wrote: >

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

jcranmer-intel added a comment. In D136568#3878104 , @Izaron wrote: > The online documentation > (https://en.cppreference.com/w/cpp/numeric/math/ilogb) says: > > 1. If the correct result is greater than INT_MAX or smaller than INT_MIN, > FE_INVALID

[PATCH] D134369: [Clang] Support constexpr builtin fmax

jcranmer-intel added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:14029-14030 + case Builtin::BI__builtin_fmaxf16: + case Builtin::BI__builtin_fmaxf128: { +APFloat RHS(0.); +if (!EvaluateFloat(E->getArg(0), Result, Info) || I think you

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Floating.h:33-34 + /// Primitive representing limits. + // static constexpr auto Min = std::numeric_limits::min(); + // static constexpr auto Max = std::numeric_limits::max(); + tbaeder

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Floating.h:27-29 + template struct Repr; + template <> struct Repr<32> { using Type = float; }; + template <> struct Repr<64> { using Type = double; }; aaron.ballman wrote: > tbaeder

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added inline comments. Comment at: clang/lib/AST/Interp/Floating.h:27-29 + template struct Repr; + template <> struct Repr<32> { using Type = float; }; + template <> struct Repr<64> { using Type = double; }; tbaeder wrote: > jcranmer-intel

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

jcranmer-intel added a comment. Some extra constexpr tests that may be interesting: - Constant expressions that produce floating point exceptions other than `FE_INEXACT`. - Subnormal values as operands, as well as operations with normal operands that produce denormal values (i.e., check for

[PATCH] D134369: [Clang] Support constexpr builtin fmax

jcranmer-intel added a comment. In D134369#3821949 , @efriedma wrote: > I think __builtin_fmax can raise a floating-point exception; in that case, it > wouldn't be constant, I think? Not sure how consistent we are about handling > that sort of thing

[PATCH] D134407: [FPEnv] Remove inaccurate comments regarding signaling NaN for isless

jcranmer-intel accepted this revision. jcranmer-intel added a comment. This revision is now accepted and ready to land. This looks fine to me, but as clang is not normally my wheelhouse, I'd still prefer to see someone else approve this as well. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

jcranmer-intel accepted this revision. jcranmer-intel added a comment. This revision is now accepted and ready to land. I'm happy with these changes. I'll let Aaron have one last crack at the wording of the documentation, in case there's any minor editorial stuff he'd like to see cleaned up.

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

jcranmer-intel added inline comments. Comment at: clang/docs/UsersManual.rst:1430 + + * ``-ffp-contract=on`` + You can add `-fdenormal-fp-math=ieee` here. Comment at: clang/docs/UsersManual.rst:1453-1455 + Note: ``DenormalFPMath`` and

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

jcranmer-intel added a comment. Some general comments on the documentation here: One of the lesser-known issues of `-ffast-math` is the fact that it (or `-funsafe-math-optimizations`) causes `crtfastmath.o` to be linked, which adds a static constructor that sets the FTZ/DAZ bits in MXCSR,

[PATCH] D132056: [HLSL] Restrict to supported targets

jcranmer-intel accepted this revision. jcranmer-intel added a comment. This revision is now accepted and ready to land. I don't necessarily know clang-specific code to have that valuable an opinion here, but I don't see anything wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

jcranmer-intel added a comment. In D127579#3588607 , @iliya-diyachkov wrote: > I think the return type information can be added in the next version of the > patch, however it should be attached to function declarations (not > definitions as it's done

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

jcranmer-intel added a comment. In D127579#3585553 , @bader wrote: > And in addition to that ISA defines types, which are not natively supported > by LLVM IR e.g. image. To represent those types clang in OpenCL language mode > emits a pointer to an

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

jcranmer-intel created this revision. Herald added a subscriber: ormris. Herald added a project: All. jcranmer-intel requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127221

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

jcranmer-intel added a comment. Apologies for the drive-by comment, but I happened to be searching for TBAA reviews after lamenting the current documentation and this popped up. > Agreed, strict aliasing violations are already a problem with the current > level of TBAA support and we regularly

[PATCH] D119291: [Clang] Add support for STDC CX_LIMITED_RANGE pragma.

jcranmer-intel created this revision. Herald added a subscriber: dexonsmith. jcranmer-intel requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This pragma is defined in the C specification. Notably, the C specification makes the "DEFAULT"

[PATCH] D119290: [Clang] Add support for -fcx-limited-range, -fcx-fortran-rules options.

jcranmer-intel created this revision. Herald added a subscriber: dang. jcranmer-intel requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. These are built on top of, and require, the complex intrinsics definitions to work with. They are

[PATCH] D119289: [Clang] Add lowering for _C complex arithmetic to complex intrinsics.

jcranmer-intel created this revision. Herald added subscribers: dexonsmith, dang, pengfei. jcranmer-intel requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This adds a new guard flag, -fuse-complex-intrinsics, that is defaulted to on for the

[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

jcranmer-intel updated this revision to Diff 215999. jcranmer-intel added a comment. I think there are use cases for having a callgraph that errs on the side of adding edges that might not exist, but I'm happy enough to leave that for a later patch. This does remind me that we need a real

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

jcranmer-intel updated this revision to Diff 215462. jcranmer-intel added a comment. I've rolled the relevant call graph analysis changes from the prior commit into this updated patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65453/new/ https://reviews.llvm.org/D65453 Files:

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

jcranmer-intel added a comment. Okay, I see the issue now. I originally developed this patch on a fork with a whole lot of extra changes, and that fork included some extra modifications to the callgraph that I had missed:

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

jcranmer-intel added a comment. The test has been passing for me. What error are you seeing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65453/new/ https://reviews.llvm.org/D65453 ___ cfe-commits

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

jcranmer-intel added a comment. No, I do not have commit access, so if you could commit it, it would be greatly appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65453/new/ https://reviews.llvm.org/D65453

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

jcranmer-intel created this revision. jcranmer-intel added a reviewer: dcoughlin. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch improves Clang call graph analysis by adding in expressions that are not found in regular function bodies, such as default arguments