mibintc updated this revision to Diff 261036. mibintc added a comment. Thanks for your review @sepavloff and @erichkeane, I have responded to your feedback and reverted the name back to reassoc which is what Serge prefers
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78827/new/ https://reviews.llvm.org/D78827 Files: clang/docs/LanguageExtensions.rst clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Basic/LangOptions.h clang/include/clang/Sema/Sema.h clang/lib/CodeGen/BackendUtil.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Parse/ParsePragma.cpp clang/lib/Sema/SemaAttr.cpp clang/test/CodeGen/fp-reassoc-pragma.cpp clang/test/Parser/pragma-fp-contract.c
Index: clang/test/Parser/pragma-fp-contract.c =================================================================== --- clang/test/Parser/pragma-fp-contract.c +++ clang/test/Parser/pragma-fp-contract.c @@ -23,3 +23,18 @@ // expected-error@+1 {{this pragma cannot appear in union declaration}} #pragma STDC FP_CONTRACT ON }; + +float fp_reassoc_fail(float a, float b) { + // CHECK-LABEL: fp_reassoc_fail + // expected-error@+2{{'#pragma clang fp' can only appear at file scope or at the start of a compound statement}} + float c = a + b; +#pragma clang fp reassoc(on) + return c - b; +} + +float fp_reassoc_no_fast(float a, float b) { +// CHECK-LABEL: fp_reassoc_no_fast +// expected-error@+1{{unexpected argument 'fast' to '#pragma clang fp reassoc'; expected 'on' or 'off'}} +#pragma clang fp reassoc(fast) + return a - b; +} Index: clang/test/CodeGen/fp-reassoc-pragma.cpp =================================================================== --- /dev/null +++ clang/test/CodeGen/fp-reassoc-pragma.cpp @@ -0,0 +1,90 @@ +// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s +// Simple case +float fp_reassoc_simple(float a, float b, float c) { +// CHECK: _Z17fp_reassoc_simplefff +// CHECK: %[[M:.+]] = fmul reassoc float %a, %b +// CHECK-NEXT: fadd reassoc float %[[M]], %c +#pragma clang fp reassoc(on) + return a * b + c; +} + +// Reassoc pragma should only apply to its scope +float fp_reassoc_scoped(float a, float b, float c) { + // CHECK: _Z17fp_reassoc_scopedfff + // CHECK: %[[M:.+]] = fmul float %a, %b + // CHECK-NEXT: fadd float %[[M]], %c + { +#pragma clang fp reassoc(on) + } + return a * b + c; +} + +// Reassoc pragma should apply to templates as well +class Foo {}; +Foo operator+(Foo, Foo); +template <typename T> +T template_reassoc(T a, T b, T c) { +#pragma clang fp reassoc(on) + return ((a + b) - c) + c; +} + +float fp_reassoc_template(float a, float b, float c) { + // CHECK: _Z19fp_reassoc_templatefff + // CHECK: %[[A1:.+]] = fadd reassoc float %a, %b + // CHECK-NEXT: %[[A2:.+]] = fsub reassoc float %[[A1]], %c + // CHECK-NEXT: fadd reassoc float %[[A2]], %c + return template_reassoc<float>(a, b, c); +} + +// File Scoping should work across functions +#pragma clang fp reassoc(on) +float fp_file_scope_on(float a, float b, float c) { + // CHECK: _Z16fp_file_scope_onfff + // CHECK: %[[M1:.+]] = fmul reassoc float %a, %c + // CHECK-NEXT: %[[M2:.+]] = fmul reassoc float %b, %c + // CHECK-NEXT: fadd reassoc float %[[M1]], %[[M2]] + return (a * c) + (b * c); +} + +// Inner pragma has precedence +float fp_file_scope_stop(float a, float b, float c) { + // CHECK: _Z18fp_file_scope_stopfff + // CHECK: %[[A:.+]] = fadd reassoc float %a, %a + // CHECK: %[[M1:.+]] = fmul float %[[A]], %c + // CHECK-NEXT: %[[M2:.+]] = fmul float %b, %c + // CHECK-NEXT: fsub float %[[M1]], %[[M2]] + a = a + a; + { +#pragma clang fp reassoc(off) + return (a * c) - (b * c); + } +} + +#pragma clang fp reassoc(off) +float fp_reassoc_off(float a, float b, float c) { + // CHECK: _Z14fp_reassoc_offfff + // CHECK: %[[D1:.+]] = fdiv float %a, %c + // CHECK-NEXT: %[[D2:.+]] = fdiv float %b, %c + // CHECK-NEXT: fadd float %[[D1]], %[[D2]] + return (a / c) + (b / c); +} + +// Takes latest flag +float fp_reassoc_many(float a, float b, float c) { +// CHECK: _Z15fp_reassoc_manyfff +// CHECK: %[[D1:.+]] = fdiv reassoc float %a, %c +// CHECK-NEXT: %[[D2:.+]] = fdiv reassoc float %b, %c +// CHECK-NEXT: fadd reassoc float %[[D1]], %[[D2]] +#pragma clang fp reassoc(off) reassoc(on) + return (a / c) + (b / c); +} + +// Pragma does not propagate through called functions +float helper_func(float a, float b, float c) { return a + b + c; } +float fp_reassoc_call_helper(float a, float b, float c) { +// CHECK: _Z22fp_reassoc_call_helperfff +// CHECK: %[[S1:.+]] = fadd float %a, %b +// CHECK-NEXT: fadd float %[[S1]], %c +#pragma clang fp reassoc(on) + return helper_func(a, b, c); +} Index: clang/lib/Sema/SemaAttr.cpp =================================================================== --- clang/lib/Sema/SemaAttr.cpp +++ clang/lib/Sema/SemaAttr.cpp @@ -985,20 +985,33 @@ } } -void Sema::ActOnPragmaFPContract(LangOptions::FPContractModeKind FPC) { +void Sema::ActOnPragmaFPContract(LangOptions::FPModeKind FPC) { switch (FPC) { - case LangOptions::FPC_On: + case LangOptions::FPM_On: CurFPFeatures.setAllowFPContractWithinStatement(); break; - case LangOptions::FPC_Fast: + case LangOptions::FPM_Fast: CurFPFeatures.setAllowFPContractAcrossStatement(); break; - case LangOptions::FPC_Off: + case LangOptions::FPM_Off: CurFPFeatures.setDisallowFPContract(); break; } } +void Sema::ActOnPragmaFPAllowReassociation(LangOptions::FPModeKind FPC) { + switch (FPC) { + default: + llvm_unreachable("invalid pragma fp reassoc kind"); + case LangOptions::FPM_On: + CurFPFeatures.setAllowAssociativeMath(true); + break; + case LangOptions::FPM_Off: + CurFPFeatures.setAllowAssociativeMath(false); + break; + } +} + void Sema::setRoundingMode(llvm::RoundingMode FPR) { CurFPFeatures.setRoundingMode(FPR); } @@ -1008,9 +1021,11 @@ } void Sema::ActOnPragmaFEnvAccess(SourceLocation Loc, - LangOptions::FEnvAccessModeKind FPC) { + LangOptions::FPModeKind FPC) { switch (FPC) { - case LangOptions::FEA_On: + default: + llvm_unreachable("invalid pragma fp fenv access kind"); + case LangOptions::FPM_On: // Verify Microsoft restriction: // You can't enable fenv_access unless precise semantics are enabled. // Precise semantics can be enabled either by the float_control @@ -1019,7 +1034,7 @@ Diag(Loc, diag::err_pragma_fenv_requires_precise); CurFPFeatures.setAllowFEnvAccess(); break; - case LangOptions::FEA_Off: + case LangOptions::FPM_Off: CurFPFeatures.setDisallowFEnvAccess(); break; } Index: clang/lib/Parse/ParsePragma.cpp =================================================================== --- clang/lib/Parse/ParsePragma.cpp +++ clang/lib/Parse/ParsePragma.cpp @@ -643,13 +643,13 @@ static_cast<tok::OnOffSwitch>( reinterpret_cast<uintptr_t>(Tok.getAnnotationValue())); - LangOptions::FPContractModeKind FPC; + LangOptions::FPModeKind FPC; switch (OOS) { case tok::OOS_ON: - FPC = LangOptions::FPC_On; + FPC = LangOptions::FPM_On; break; case tok::OOS_OFF: - FPC = LangOptions::FPC_Off; + FPC = LangOptions::FPM_Off; break; case tok::OOS_DEFAULT: FPC = getLangOpts().getDefaultFPContractMode(); @@ -682,16 +682,16 @@ static_cast<tok::OnOffSwitch>( reinterpret_cast<uintptr_t>(Tok.getAnnotationValue())); - LangOptions::FEnvAccessModeKind FPC; + LangOptions::FPModeKind FPC; switch (OOS) { case tok::OOS_ON: - FPC = LangOptions::FEA_On; + FPC = LangOptions::FPM_On; break; case tok::OOS_OFF: - FPC = LangOptions::FEA_Off; + FPC = LangOptions::FPM_Off; break; case tok::OOS_DEFAULT: // FIXME: Add this cli option when it makes sense. - FPC = LangOptions::FEA_Off; + FPC = LangOptions::FPM_Off; break; } @@ -2828,7 +2828,7 @@ namespace { /// Used as the annotation value for tok::annot_pragma_fp. struct TokFPAnnotValue { - enum FlagKinds { Contract }; + enum FlagKinds { Contract, Reassoc }; enum FlagValues { On, Off, Fast }; FlagKinds FlagKind; @@ -2856,6 +2856,7 @@ llvm::StringSwitch<llvm::Optional<TokFPAnnotValue::FlagKinds>>( OptionInfo->getName()) .Case("contract", TokFPAnnotValue::Contract) + .Case("reassoc", TokFPAnnotValue::Reassoc) .Default(None); if (!FlagKind) { PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_option) @@ -2873,7 +2874,8 @@ if (Tok.isNot(tok::identifier)) { PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_argument) - << PP.getSpelling(Tok) << OptionInfo->getName(); + << PP.getSpelling(Tok) << OptionInfo->getName() + << (FlagKind == TokFPAnnotValue::Reassoc); return; } const IdentifierInfo *II = Tok.getIdentifierInfo(); @@ -2886,9 +2888,17 @@ .Case("fast", TokFPAnnotValue::Fast) .Default(llvm::None); - if (!FlagValue) { + if (FlagKind == TokFPAnnotValue::Reassoc) { + if (!FlagValue || FlagValue == TokFPAnnotValue::Fast) { + PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_argument) + << PP.getSpelling(Tok) << OptionInfo->getName() + << (FlagKind == TokFPAnnotValue::Reassoc); + return; + } + } else if (!FlagValue) { PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_argument) - << PP.getSpelling(Tok) << OptionInfo->getName(); + << PP.getSpelling(Tok) << OptionInfo->getName() + << (FlagKind == TokFPAnnotValue::Reassoc); return; } PP.Lex(Tok); @@ -2930,20 +2940,23 @@ auto *AnnotValue = reinterpret_cast<TokFPAnnotValue *>(Tok.getAnnotationValue()); - LangOptions::FPContractModeKind FPC; + LangOptions::FPModeKind FPC; switch (AnnotValue->FlagValue) { case TokFPAnnotValue::On: - FPC = LangOptions::FPC_On; + FPC = LangOptions::FPM_On; break; case TokFPAnnotValue::Fast: - FPC = LangOptions::FPC_Fast; + FPC = LangOptions::FPM_Fast; break; case TokFPAnnotValue::Off: - FPC = LangOptions::FPC_Off; + FPC = LangOptions::FPM_Off; break; } - Actions.ActOnPragmaFPContract(FPC); + if (AnnotValue->FlagKind == TokFPAnnotValue::Reassoc) + Actions.ActOnPragmaFPAllowReassociation(FPC); + else + Actions.ActOnPragmaFPContract(FPC); ConsumeAnnotationToken(); } Index: clang/lib/Frontend/CompilerInvocation.cpp =================================================================== --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -2318,7 +2318,7 @@ Opts.AltiVec = 0; Opts.ZVector = 0; Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::None); - Opts.setDefaultFPContractMode(LangOptions::FPC_On); + Opts.setDefaultFPContractMode(LangOptions::FPM_On); Opts.NativeHalfType = 1; Opts.NativeHalfArgsAndReturns = 1; Opts.OpenCLCPlusPlus = Opts.CPlusPlus; @@ -2338,7 +2338,7 @@ Opts.CUDA = IK.getLanguage() == Language::CUDA || Opts.HIP; if (Opts.CUDA) // Set default FP_CONTRACT to FAST. - Opts.setDefaultFPContractMode(LangOptions::FPC_Fast); + Opts.setDefaultFPContractMode(LangOptions::FPM_Fast); Opts.RenderScript = IK.getLanguage() == Language::RenderScript; if (Opts.RenderScript) { @@ -3204,11 +3204,11 @@ if (Arg *A = Args.getLastArg(OPT_ffp_contract)) { StringRef Val = A->getValue(); if (Val == "fast") - Opts.setDefaultFPContractMode(LangOptions::FPC_Fast); + Opts.setDefaultFPContractMode(LangOptions::FPM_Fast); else if (Val == "on") - Opts.setDefaultFPContractMode(LangOptions::FPC_On); + Opts.setDefaultFPContractMode(LangOptions::FPM_On); else if (Val == "off") - Opts.setDefaultFPContractMode(LangOptions::FPC_Off); + Opts.setDefaultFPContractMode(LangOptions::FPM_Off); else Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val; } Index: clang/lib/CodeGen/BackendUtil.cpp =================================================================== --- clang/lib/CodeGen/BackendUtil.cpp +++ clang/lib/CodeGen/BackendUtil.cpp @@ -450,15 +450,15 @@ // Set FP fusion mode. switch (LangOpts.getDefaultFPContractMode()) { - case LangOptions::FPC_Off: + case LangOptions::FPM_Off: // Preserve any contraction performed by the front-end. (Strict performs // splitting of the muladd intrinsic in the backend.) Options.AllowFPOpFusion = llvm::FPOpFusion::Standard; break; - case LangOptions::FPC_On: + case LangOptions::FPM_On: Options.AllowFPOpFusion = llvm::FPOpFusion::Standard; break; - case LangOptions::FPC_Fast: + case LangOptions::FPM_Fast: Options.AllowFPOpFusion = llvm::FPOpFusion::Fast; break; } Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -9617,12 +9617,15 @@ /// ActOnPragmaFPContract - Called on well formed /// \#pragma {STDC,OPENCL} FP_CONTRACT and /// \#pragma clang fp contract - void ActOnPragmaFPContract(LangOptions::FPContractModeKind FPC); + void ActOnPragmaFPContract(LangOptions::FPModeKind FPC); + + /// Called on well formed + /// \#pragma clang fp reassoc + void ActOnPragmaFPAllowReassociation(LangOptions::FPModeKind FPC); /// ActOnPragmaFenvAccess - Called on well formed /// \#pragma STDC FENV_ACCESS - void ActOnPragmaFEnvAccess(SourceLocation Loc, - LangOptions::FEnvAccessModeKind FPC); + void ActOnPragmaFEnvAccess(SourceLocation Loc, LangOptions::FPModeKind FPC); /// Called to set rounding mode for floating point operations. void setRoundingMode(llvm::RoundingMode); Index: clang/include/clang/Basic/LangOptions.h =================================================================== --- clang/include/clang/Basic/LangOptions.h +++ clang/include/clang/Basic/LangOptions.h @@ -174,22 +174,15 @@ Swift4_1, }; - enum FPContractModeKind { - // Form fused FP ops only where result will not be affected. - FPC_Off, + enum FPModeKind { + // Disable the floating point pragma + FPM_Off, - // Form fused FP ops according to FP_CONTRACT rules. - FPC_On, + // Enable the floating point pragma + FPM_On, - // Aggressively fuse FP ops (E.g. FMA). - FPC_Fast - }; - - // TODO: merge FEnvAccessModeKind and FPContractModeKind - enum FEnvAccessModeKind { - FEA_Off, - - FEA_On + // (Only for fp contract) Aggressively fuse FP ops (E.g. FMA). + FPM_Fast }; /// Alias for RoundingMode::NearestTiesToEven. @@ -380,7 +373,7 @@ public: FPOptions() - : fp_contract(LangOptions::FPC_Off), fenv_access(LangOptions::FEA_Off), + : fp_contract(LangOptions::FPM_Off), fenv_access(LangOptions::FPM_Off), rounding(LangOptions::FPR_ToNearest), exceptions(LangOptions::FPE_Ignore), allow_reassoc(0), no_nans(0), no_infs(0), no_signed_zeros(0), allow_reciprocal(0), approx_func(0) {} @@ -390,7 +383,7 @@ explicit FPOptions(const LangOptions &LangOpts) : fp_contract(LangOpts.getDefaultFPContractMode()), - fenv_access(LangOptions::FEA_Off), + fenv_access(LangOptions::FPM_Off), rounding(static_cast<unsigned>(LangOpts.getFPRoundingMode())), exceptions(LangOpts.getFPExceptionMode()), allow_reassoc(LangOpts.FastMath || LangOpts.AllowFPReassoc), @@ -415,30 +408,26 @@ bool requiresTrailingStorage(const LangOptions &LO); bool allowFPContractWithinStatement() const { - return fp_contract == LangOptions::FPC_On; + return fp_contract == LangOptions::FPM_On; } bool allowFPContractAcrossStatement() const { - return fp_contract == LangOptions::FPC_Fast; + return fp_contract == LangOptions::FPM_Fast; } void setAllowFPContractWithinStatement() { - fp_contract = LangOptions::FPC_On; + fp_contract = LangOptions::FPM_On; } void setAllowFPContractAcrossStatement() { - fp_contract = LangOptions::FPC_Fast; + fp_contract = LangOptions::FPM_Fast; } - void setDisallowFPContract() { fp_contract = LangOptions::FPC_Off; } + void setDisallowFPContract() { fp_contract = LangOptions::FPM_Off; } - bool allowFEnvAccess() const { - return fenv_access == LangOptions::FEA_On; - } + bool allowFEnvAccess() const { return fenv_access == LangOptions::FPM_On; } - void setAllowFEnvAccess() { - fenv_access = LangOptions::FEA_On; - } + void setAllowFEnvAccess() { fenv_access = LangOptions::FPM_On; } void setFPPreciseEnabled(bool Value) { if (Value) { @@ -452,7 +441,7 @@ } } - void setDisallowFEnvAccess() { fenv_access = LangOptions::FEA_Off; } + void setDisallowFEnvAccess() { fenv_access = LangOptions::FPM_Off; } RoundingMode getRoundingMode() const { return static_cast<RoundingMode>(rounding); @@ -502,8 +491,8 @@ /// Used with getAsOpaqueInt() to manage the float_control pragma stack. void getFromOpaqueInt(unsigned I) { - fp_contract = (static_cast<LangOptions::FPContractModeKind>(I & 3)); - fenv_access = (static_cast<LangOptions::FEnvAccessModeKind>((I >> 2) & 1)); + fp_contract = (static_cast<LangOptions::FPModeKind>(I & 3)); + fenv_access = (static_cast<LangOptions::FPModeKind>((I >> 2) & 1)); rounding = static_cast<unsigned>(static_cast<RoundingMode>((I >> 3) & 7)); exceptions = (static_cast<LangOptions::FPExceptionModeKind>((I >> 6) & 3)); allow_reassoc = ((I >> 8) & 1); Index: clang/include/clang/Basic/LangOptions.def =================================================================== --- clang/include/clang/Basic/LangOptions.def +++ clang/include/clang/Basic/LangOptions.def @@ -268,7 +268,7 @@ LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating point constants as single precision constants") LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math") /// FP_CONTRACT mode (on/off/fast). -ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") +ENUM_LANGOPT(DefaultFPContractMode, FPModeKind, 2, FPM_Off, "FP contraction type") ENUM_LANGOPT(FPRoundingMode, RoundingMode, 3, RoundingMode::NearestTiesToEven, "FP Rounding Mode type") ENUM_LANGOPT(FPExceptionMode, FPExceptionModeKind, 2, FPE_Ignore, "FP Exception Behavior Mode type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment") Index: clang/include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticParseKinds.td +++ clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1332,10 +1332,12 @@ "pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute">; def err_pragma_fp_invalid_option : Error< - "%select{invalid|missing}0 option%select{ %1|}0; expected contract">; + "%select{invalid|missing}0 option%select{ %1|}0; expected 'contract' or 'reassoc'">; def err_pragma_fp_invalid_argument : Error< "unexpected argument '%0' to '#pragma clang fp %1'; " - "expected 'on', 'fast' or 'off'">; + "%select{" + "expected 'fast' or 'on' or 'off'|" + "expected 'on' or 'off'}2">; def err_pragma_invalid_keyword : Error< "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">; Index: clang/docs/LanguageExtensions.rst =================================================================== --- clang/docs/LanguageExtensions.rst +++ clang/docs/LanguageExtensions.rst @@ -3173,10 +3173,18 @@ compound statement, the pragma is active within the scope of the compound statement. -Currently, only FP contraction can be controlled with the pragma. ``#pragma -clang fp contract`` specifies whether the compiler should contract a multiply -and an addition (or subtraction) into a fused FMA operation when supported by -the target. +Both floating point reassociation and floating point contraction can be +controlled with this pragma. +``#pragma clang fp reassoc`` allows control over the reassociation +of floating point expressions. When enabled, this pragma allows the expression +``x + (y + z)`` to be reassociated as ``(x + y) + z``. +This can be useful when reassociation is otherwise +enabled for the translation unit with the ``-fassociative-math`` flag. +The pragma can take two values: ``on`` and ``off``. + +``#pragma clang fp contract`` specifies whether the compiler should +contract a multiply and an addition (or subtraction) into a fused FMA +operation when supported by the target. The pragma can take three values: ``on``, ``fast`` and ``off``. The ``on`` option is identical to using ``#pragma STDC FP_CONTRACT(ON)`` and it allows
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits