[PATCH] D57220: Test fix for isViableInline remark message
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57220/new/ https://reviews.llvm.org/D57220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds
anemet added a comment. Actually this has been failing for 8 hours. So reverted in r349117. Also reverted your attempt to update the test. It wasn't updating the right test: r349118 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds
anemet added a comment. This caused: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/56120/consoleFull#1420996271a1ca8a51-895e-46c6-af87-ce24fa4cd561 Please fix or revert. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49348: Harden/relax clang/test/CodeGen/opt-record-MIR.c test
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang https://reviews.llvm.org/D49348 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a comment. Sorted these out in https://reviews.llvm.org/rL319576, https://reviews.llvm.org/rL319577 and https://reviews.llvm.org/rL319578. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a comment. Looks like it's a test problem. When I tweak the sample profile file according to https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, I do get hotness on the remarks. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a subscriber: davide. anemet added a comment. @modocache, @davide, are you guys sure this feature is working? The test does not actually check whether hotness is included in the remarks and when I run it manually they are missing. In https://reviews.llvm.org/D40678, I am filtering out remarks with no hotness when any threshold is set all the remarks are filtered out in this new test. So either the test is incorrect or somehow with sample-based profiling we don't get hotness info. Any ideas? I am inclined to just remove this test for now and file a bug to fix this in order to unblock https://reviews.llvm.org/D40678. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D37196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet accepted this revision. anemet added a comment. Still looks good. Comment at: lib/IR/LLVMContext.cpp:332 +} \ No newline at end of file Has this version of the diff been clang-formatted? https://reviews.llvm.org/D33514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet added a comment.
In https://reviews.llvm.org/D37196#868320, @vivekvpandya wrote:
> > Why? That was inside BackendConsumer.
>
> I was getting incomplete type error.
You may have to move the class declaration of ClangDiagnosticHandler before the
BackendConsumer and move the definition of
ClangDiagnosticHandler::handleDiagnositic out of line since that is where you
use BackendConsumer. Something like:
class BackendConsumer;
class ClangDiagnosticHandler {
public:
... // as before
bool handleDiagnostics(const DiagnosticInfo ) override;
...
private:
...
}
class BackendConsumer {
...
}
bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo ) {
...
}
>> You can just save the old DiagHandler object instead.
>
> I don't think now we need to do that as per my understanding CodeGen i.e
> emitting LLVM IR is last phase in clang which will pass control to
> LLVMContext. LLVMContext will have Base class of DiagnosticHandler which will
> not handle diagnostic and return false so LLVMContext::diagnose() will print
> diagnostic with
>
> DiagnosticPrinterRawOStream DP(errs());
>
> errs() << getDiagnosticMessagePrefix(DI.getSeverity()) << ": ";
> DI.print(DP);
> errs() << "\n";
>
>
> and clang's diagnostic handler also does similar thing so if we keep
> ClangDiagnosticHandler pointer in LLVMContext it should not break.
That would have been true even before and we still restored it, no? I guess I
am not sure why we used to restore this so I would just do it regardless and
not change functionality.
If you want we can separately take a look whether we need to restore these
original handlers.
https://reviews.llvm.org/D37196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet added inline comments. Comment at: lib/CodeGen/CodeGenAction.cpp:882-883 BEConsumer = Result.get(); - + VMContext->setDiagnosticHandler(llvm::make_unique( + CI.getCodeGenOpts(), Result.get())); // Enable generating macro debug info only when debug info is not disabled and vivekvpandya wrote: > anemet wrote: > > Any reason you moved where we set this up? > > > # At older place I was not able to define ClangDiagnosticHandler class as > it will require definition of BackendComsumer and vice versa. > # and this seems to be appropriate place to create and tie > DiagnosticHandler to LLVMContext > > > But I am not sure about why old diagnostic handler was tied back to context > that is why I wanted someone from clang to look at this. > > > At older place I was not able to define ClangDiagnosticHandler class as it > will require definition of BackendComsumer and vice versa. Why? That was inside BackendConsumer. > But I am not sure about why old diagnostic handler was tied back to context > that is why I wanted someone from clang to look at this. You can just save the old DiagHandler object instead. https://reviews.llvm.org/D37196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM with the nits below. Thanks! Comment at: include/llvm/IR/DiagnosticHandler.h:1 +//===- DiagnosticHandler.cpp - DiagnosticHandler class for LLVM -*- C++ -*-===// +// DiagnosticHandler.h Comment at: lib/IR/LLVMContext.cpp:197 return Remark->isEnabled(); - return true; Remove this whitespace change Comment at: lib/IR/LLVMContextImpl.cpp:25 LLVMContextImpl::LLVMContextImpl(LLVMContext ) - : VoidTy(C, Type::VoidTyID), + : DiagHandler(llvm::make_unique(nullptr)), +VoidTy(C, Type::VoidTyID), No need to pass nullptr here. https://reviews.llvm.org/D33514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet added a comment.
Please clean this up as well (don't have commented-out lines) so that it's
ready to go with the LLVM patch.
Comment at: lib/CodeGen/CodeGenAction.cpp:302-305
static void DiagnosticHandler(const llvm::DiagnosticInfo ,
void *Context) {
((BackendConsumer *)Context)->DiagnosticHandlerImpl(DI);
}
Remove this.
Comment at: lib/CodeGen/CodeGenAction.cpp:761
+public:
+ ClangDiagnosticHandler(const CodeGenOptions , void *DiagContext)
+ : DiagnosticHandler(DiagContext), CodeGenOpts(CGOpts) {}
Again don't overload DiagContext as such. Have a dedicated field for the
BackendConsumer.
Comment at: lib/CodeGen/CodeGenAction.cpp:882-883
BEConsumer = Result.get();
-
+ VMContext->setDiagnosticHandler(llvm::make_unique(
+ CI.getCodeGenOpts(), Result.get()));
// Enable generating macro debug info only when debug info is not disabled
and
Any reason you moved where we set this up?
https://reviews.llvm.org/D37196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
anemet added a comment.
Only minor things at this point. This is very close now.
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:81
/// detected by the user.
- bool allowExtraAnalysis() const {
-// For now, only allow this with -fsave-optimization-record since the
-Rpass
-// options are handled in the front-end.
-return F->getContext().getDiagnosticsOutputFile();
+ bool allowExtraAnalysis(StringRef &) const {
+return (F->getContext().getDiagnosticsOutputFile() ||
Why rvalue reference, you should just take this by value. There is no
ownership transfer here.
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:84
+F->getContext().getDiagHandler()->isAnyRemarkEnabled(
+std::move(PassName)));
}
No std::move here. You have more of this later.
Comment at: include/llvm/IR/DiagnosticHandler.h:13
+
+#include "llvm/Support/Regex.h"
+
I don't think you need this.
Comment at: include/llvm/IR/DiagnosticHandler.h:18
+
+struct DiagnosticHandler {
+public:
Please add a comment before the class.
Comment at: include/llvm/IR/DiagnosticHandler.h:19
+struct DiagnosticHandler {
+public:
+ void *DiagnosticContext = nullptr;
No need for public for struct.
Comment at: include/llvm/IR/DiagnosticHandler.h:27
+
+ /// DiagHandlerCallback is settable from the C API and base implimentation
+ /// of DiagnosticHandler will call it from handleDiagnostics(). Any derived
implementation
Comment at: include/llvm/IR/DiagnosticHandler.h:45
+
+ /// checks if remark requested with -pass-remarks-analysis, override
+ /// to provide different implementation
Capitalize first word and end with period; comments are full sentences.
Comment at: include/llvm/IR/DiagnosticHandler.h:57
+
+ /// checks if remark requested with -pass-remarks{-missed/-analysis}
+ bool isAnyRemarkEnabled(StringRef &) const {
I would drop the flag names here since if those are overridden this is not
true. Just say "Return true if any remark type is enabled."
Comment at: lib/IR/DiagnosticInfo.cpp:47-65
/// \brief Regular expression corresponding to the value given in one of the
/// -pass-remarks* command line flags. Passes whose name matches this regexp
/// will emit a diagnostic via ORE->emit(...);
struct PassRemarksOpt {
std::shared_ptr Pattern;
void operator=(const std::string ) {
Is this still used here?
Comment at: lib/LTO/LTOCodeGenerator.cpp:667-668
-return Context.setDiagnosticHandler(nullptr, nullptr);
- // Register the LTOCodeGenerator stub in the LLVMContext to forward the
- // diagnostic to the external DiagHandler.
- Context.setDiagnosticHandler(LTOCodeGenerator::DiagnosticHandler, this,
I think that this comment still applies.
Comment at: tools/llvm-lto/llvm-lto.cpp:39-72
+static std::string CurrentActivity;
+namespace {
+struct LLVMLTODiagnosticHandler : public DiagnosticHandler {
+ bool handleDiagnostics(const DiagnosticInfo ) override {
+raw_ostream = errs();
+OS << "llvm-lto: ";
+switch (DI.getSeverity()) {
Don't move this code unless you have to. The diff is easier to read that way.
https://reviews.llvm.org/D33514
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36949: [clang] Fix tests for Emitting Single Inline Remark
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D36949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. Looks great, thank you! https://reviews.llvm.org/D34868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold
anemet added a comment.
Great!
Comment at: docs/UsersManual.rst:330-332
This option, which defaults to off, controls whether Clang prints the
profile hotness associated with a diagnostics in the presence of
profile-guided optimization information. This is currently supported with
While you're here, could you please update this information. This option is
implied by -fsave-optimization-record so it's not generally true that it's off
by default.
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:198-203
def warn_drv_fdiagnostics_show_hotness_requires_pgo : Warning<
"argument '-fdiagnostics-show-hotness' requires profile-guided optimization
information">,
InGroup;
+def warn_drv_fdiagnostics_hotness_threshold_requires_pgo : Warning<
+ "argument '-fdiagnostics-hotness-threshold=' requires profile-guided
optimization information">,
+ InGroup;
Can you merge these two by taking the name of the option as the parameter %0?
Comment at: include/clang/Driver/Options.td:728
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent optimization remarks from being output if they do not
have at least this hotness value">;
def fdiagnostics_show_option : Flag<["-"], "fdiagnostics-show-option">,
Group,
Switch "hotness value" to "profile count"?
Comment at: lib/Frontend/CompilerInvocation.cpp:912-914
+ if (Opts.DiagnosticsWithHotness && !UsingProfile) {
Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
}
There is no {} for single statement blocks.
Comment at: test/Frontend/optimization-remark-with-hotness.c:9-20
// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
// RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -verify
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -verify
// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
I think that one of these (or additional tests) should also cover the default
value (i.e. omitted -fdiagnostics-hotness-threshold=).
https://reviews.llvm.org/D34868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34865: [ORE] Use LLVM's "diagnostics hotness" spelling
anemet accepted this revision. anemet added a comment. This revision is now accepted and ready to land. LGTM, thanks! https://reviews.llvm.org/D34865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
anemet added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:32 +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ modocache wrote: > Ideally, I'd like for this test to be identical to the ones that use > `-verify` above, save for using `-fprofile-sample-use`. However I couldn't > figure out how to write `optimization-remark-with-hotness-sample.proftext` > such that it would produce hotness info for each of the functions. I'm able > to confirm, using real sampling from another program of mine, that remarks > using AutoFDO data do indeed show hotness, but this test does not verify this > in its current state. > > Any advice here? I wrote `optimization-remark-with-hotness-sample.proftext` > based on the format specified in > https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, but > maybe it's missing something that would allow hotness to be displayed? I don't have any immediate ideas since I am not familiar with sample-based profiling unfortunately. I will study this later unless you beat me to it or David has some ideas. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
anemet added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:1-34 +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata Why don't you put the sampling test right under the instrumented test. Also I don't think we want a separate --check-prefix for it. It should just use the default CHECK since the entire point is that the two should be identical. In other words, please check that we don't warn on either case. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32301: Don't pass FPOpFusion::Strict to the backend
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300858: Don't pass FPOpFusion::Strict to the backend
(authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D32301?vs=95979=95982#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32301
Files:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/fp-contract-on-asm.c
Index: cfe/trunk/test/CodeGen/fp-contract-on-asm.c
===
--- cfe/trunk/test/CodeGen/fp-contract-on-asm.c
+++ cfe/trunk/test/CodeGen/fp-contract-on-asm.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -O3 -triple=aarch64-apple-ios -S -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+#pragma STDC FP_CONTRACT ON
+// CHECK-LABEL: fma_test1:
+// CHECK: fmadd
+ float x = a * b + c;
+ return x;
+}
+
+float fma_test2(float a, float b, float c) {
+// CHECK-LABEL: fma_test2:
+// CHECK: fmul
+// CHECK: fadd
+ float x = a * b + c;
+ return x;
+}
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -369,7 +369,9 @@
// Set FP fusion mode.
switch (LangOpts.getDefaultFPContractMode()) {
case LangOptions::FPC_Off:
-Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
+// Preserve any contraction performed by the front-end. (Strict performs
+// splitting of the muladd instrinsic in the backend.)
+Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
break;
case LangOptions::FPC_On:
Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
Index: cfe/trunk/test/CodeGen/fp-contract-on-asm.c
===
--- cfe/trunk/test/CodeGen/fp-contract-on-asm.c
+++ cfe/trunk/test/CodeGen/fp-contract-on-asm.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -O3 -triple=aarch64-apple-ios -S -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+#pragma STDC FP_CONTRACT ON
+// CHECK-LABEL: fma_test1:
+// CHECK: fmadd
+ float x = a * b + c;
+ return x;
+}
+
+float fma_test2(float a, float b, float c) {
+// CHECK-LABEL: fma_test2:
+// CHECK: fmul
+// CHECK: fadd
+ float x = a * b + c;
+ return x;
+}
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -369,7 +369,9 @@
// Set FP fusion mode.
switch (LangOpts.getDefaultFPContractMode()) {
case LangOptions::FPC_Off:
-Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
+// Preserve any contraction performed by the front-end. (Strict performs
+// splitting of the muladd instrinsic in the backend.)
+Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
break;
case LangOptions::FPC_On:
Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32301: Don't pass FPOpFusion::Strict to the backend
anemet created this revision.
This restores the behavior prior to https://reviews.llvm.org/D31167 where the
code-gen default was
FPC_On which mapped to FPOpFusion::Standard. After merging the FE
state (on/off) and the code-gen state (on/fast/off), the default became off to
match the front-end.
In other words, the front-end controls when to fuse along the language
standards and the backend shouldn't override this by splitting fused
intrinsics as FPOpFusion::Strict would imply.
https://reviews.llvm.org/D32301
Files:
lib/CodeGen/BackendUtil.cpp
test/CodeGen/fp-contract-on-asm.c
Index: test/CodeGen/fp-contract-on-asm.c
===
--- /dev/null
+++ test/CodeGen/fp-contract-on-asm.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -O3 -triple=aarch64-apple-ios -S -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+#pragma STDC FP_CONTRACT ON
+// CHECK-LABEL: fma_test1:
+// CHECK: fmadd
+ float x = a * b + c;
+ return x;
+}
+
+float fma_test2(float a, float b, float c) {
+// CHECK-LABEL: fma_test2:
+// CHECK: fmul
+// CHECK: fadd
+ float x = a * b + c;
+ return x;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -372,7 +372,9 @@
// Set FP fusion mode.
switch (LangOpts.getDefaultFPContractMode()) {
case LangOptions::FPC_Off:
-Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
+// Preserve any contraction performed by the front-end. (Strict performs
+// splitting of the muladd instrinsic in the backend.)
+Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
break;
case LangOptions::FPC_On:
Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
Index: test/CodeGen/fp-contract-on-asm.c
===
--- /dev/null
+++ test/CodeGen/fp-contract-on-asm.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -O3 -triple=aarch64-apple-ios -S -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+#pragma STDC FP_CONTRACT ON
+// CHECK-LABEL: fma_test1:
+// CHECK: fmadd
+ float x = a * b + c;
+ return x;
+}
+
+float fma_test2(float a, float b, float c) {
+// CHECK-LABEL: fma_test2:
+// CHECK: fmul
+// CHECK: fadd
+ float x = a * b + c;
+ return x;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -372,7 +372,9 @@
// Set FP fusion mode.
switch (LangOpts.getDefaultFPContractMode()) {
case LangOptions::FPC_Off:
-Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
+// Preserve any contraction performed by the front-end. (Strict performs
+// splitting of the muladd instrinsic in the backend.)
+Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
break;
case LangOptions::FPC_On:
Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments.
Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220
+/// \brief FP_CONTRACT mode (on/off/fast).
+ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP
contraction type")
LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
hfinkel wrote:
> anemet wrote:
> > hfinkel wrote:
> > > yaxunl wrote:
> > > > hfinkel wrote:
> > > > > yaxunl wrote:
> > > > > > anemet wrote:
> > > > > > > yaxunl wrote:
> > > > > > > > anemet wrote:
> > > > > > > > > yaxunl wrote:
> > > > > > > > > > This change seemed to cause some performance degradations
> > > > > > > > > > in some OpenCL applications.
> > > > > > > > > >
> > > > > > > > > > This option used to be on by default. Now it is off by
> > > > > > > > > > default.
> > > > > > > > > >
> > > > > > > > > > There are applications do separated compile/link/codegen
> > > > > > > > > > stages. In the codegen stage, clang is invoked with .bc as
> > > > > > > > > > input, therefore this option is off by default, whereas it
> > > > > > > > > > was on by default before this change.
> > > > > > > > > >
> > > > > > > > > > Is there any reason not to keep the original behavior?
> > > > > > > > > >
> > > > > > > > > > Thanks.
> > > > > > > > > > This change seemed to cause some performance degradations
> > > > > > > > > > in some OpenCL applications.
> > > > > > > > > >
> > > > > > > > > > This option used to be on by default. Now it is off by
> > > > > > > > > > default.
> > > > > > > > >
> > > > > > > > > It's always been off. It was set to fast for CUDA which
> > > > > > > > > should still be the case. See the changes further down on
> > > > > > > > > the patch.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > There are applications do separated compile/link/codegen
> > > > > > > > > > stages. In the codegen stage, clang is invoked with .bc as
> > > > > > > > > > input, therefore this option is off by default, whereas it
> > > > > > > > > > was on by default before this change.
> > > > > > > > > >
> > > > > > > > > > Is there any reason not to keep the original behavior?
> > > > > > > > >
> > > > > > > > > Sorry but I am not sure what changed, can you elaborate on
> > > > > > > > > what you're doing and how things used to work for you?
> > > > > > > > Before your change, -ffp-contract was a codegen option defined
> > > > > > > > as
> > > > > > > >
> > > > > > > >
> > > > > > > > ```
> > > > > > > > ENUM_CODEGENOPT(FPContractMode, FPContractModeKind, 2, FPC_On)
> > > > > > > > ```
> > > > > > > >
> > > > > > > > therefore the default value as on. After your change, it
> > > > > > > > becomes off by default.
> > > > > > > No. -ffp-contract=on was a FE option and -ffp-contract=fast was a
> > > > > > > backend option. I still don't understand your use-case. Can you
> > > > > > > include a small testcase how this used to work before?
> > > > > > -ffp-contract=on used to be a codegen option before this change. In
> > > > > > CodeGen/BackendUtil.cpp, before this change:
> > > > > >
> > > > > > ```
> > > > > > switch (CodeGenOpts.getFPContractMode()) {
> > > > > > switch (LangOpts.getDefaultFPContractMode()) {
> > > > > > case LangOptions::FPC_Off:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
> > > > > > break;
> > > > > > case LangOptions::FPC_On:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
> > > > > > break;
> > > > > > case LangOptions::FPC_Fast:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
> > > > > > break;
> > > > > > }
> > > > > > ```
> > > > > > By default, -fp-contract=on, which results in
> > > > > > llvm::FPOpFusion::Standard in the backend. This options allows
> > > > > > backend to translate llvm.fmuladd to FMA machine instructions.
> > > > > >
> > > > > > After this change, it becomes:
> > > > > >
> > > > > > ```
> > > > > > switch (LangOpts.getDefaultFPContractMode()) {
> > > > > > case LangOptions::FPC_Off:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
> > > > > > break;
> > > > > > case LangOptions::FPC_On:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
> > > > > > break;
> > > > > > case LangOptions::FPC_Fast:
> > > > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
> > > > > > break;
> > > > > > }
> > > > > > ```
> > > > > > now by default -ffp-contract=off, which results in
> > > > > > llvm::FPOpFusion::Strict in the backend. This option disables
> > > > > > backend translating llvm.fmuladd to FMA machine instructions in
> > > > > > certain situations.
> > > > > >
> > > > > > A simple lit test is as follows:
> > > > > >
> > > > > >
> > > > > > ```
> > > > > > ; RUN: %clang_cc1 -triple amdgcn -S -o - %s| FileCheck %s
> > > > > >
> > > > > > define amdgpu_kernel void @f(double addrspace(1)* nocapture %out,
> > > > > > double %a, double %b, double
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments.
Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220
+/// \brief FP_CONTRACT mode (on/off/fast).
+ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP
contraction type")
LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
hfinkel wrote:
> yaxunl wrote:
> > hfinkel wrote:
> > > yaxunl wrote:
> > > > anemet wrote:
> > > > > yaxunl wrote:
> > > > > > anemet wrote:
> > > > > > > yaxunl wrote:
> > > > > > > > This change seemed to cause some performance degradations in
> > > > > > > > some OpenCL applications.
> > > > > > > >
> > > > > > > > This option used to be on by default. Now it is off by default.
> > > > > > > >
> > > > > > > > There are applications do separated compile/link/codegen
> > > > > > > > stages. In the codegen stage, clang is invoked with .bc as
> > > > > > > > input, therefore this option is off by default, whereas it was
> > > > > > > > on by default before this change.
> > > > > > > >
> > > > > > > > Is there any reason not to keep the original behavior?
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > > This change seemed to cause some performance degradations in
> > > > > > > > some OpenCL applications.
> > > > > > > >
> > > > > > > > This option used to be on by default. Now it is off by default.
> > > > > > >
> > > > > > > It's always been off. It was set to fast for CUDA which should
> > > > > > > still be the case. See the changes further down on the patch.
> > > > > > >
> > > > > > > >
> > > > > > > > There are applications do separated compile/link/codegen
> > > > > > > > stages. In the codegen stage, clang is invoked with .bc as
> > > > > > > > input, therefore this option is off by default, whereas it was
> > > > > > > > on by default before this change.
> > > > > > > >
> > > > > > > > Is there any reason not to keep the original behavior?
> > > > > > >
> > > > > > > Sorry but I am not sure what changed, can you elaborate on what
> > > > > > > you're doing and how things used to work for you?
> > > > > > Before your change, -ffp-contract was a codegen option defined as
> > > > > >
> > > > > >
> > > > > > ```
> > > > > > ENUM_CODEGENOPT(FPContractMode, FPContractModeKind, 2, FPC_On)
> > > > > > ```
> > > > > >
> > > > > > therefore the default value as on. After your change, it becomes
> > > > > > off by default.
> > > > > No. -ffp-contract=on was a FE option and -ffp-contract=fast was a
> > > > > backend option. I still don't understand your use-case. Can you
> > > > > include a small testcase how this used to work before?
> > > > -ffp-contract=on used to be a codegen option before this change. In
> > > > CodeGen/BackendUtil.cpp, before this change:
> > > >
> > > > ```
> > > > switch (CodeGenOpts.getFPContractMode()) {
> > > > switch (LangOpts.getDefaultFPContractMode()) {
> > > > case LangOptions::FPC_Off:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
> > > > break;
> > > > case LangOptions::FPC_On:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
> > > > break;
> > > > case LangOptions::FPC_Fast:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
> > > > break;
> > > > }
> > > > ```
> > > > By default, -fp-contract=on, which results in
> > > > llvm::FPOpFusion::Standard in the backend. This options allows backend
> > > > to translate llvm.fmuladd to FMA machine instructions.
> > > >
> > > > After this change, it becomes:
> > > >
> > > > ```
> > > > switch (LangOpts.getDefaultFPContractMode()) {
> > > > case LangOptions::FPC_Off:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
> > > > break;
> > > > case LangOptions::FPC_On:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
> > > > break;
> > > > case LangOptions::FPC_Fast:
> > > > Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
> > > > break;
> > > > }
> > > > ```
> > > > now by default -ffp-contract=off, which results in
> > > > llvm::FPOpFusion::Strict in the backend. This option disables backend
> > > > translating llvm.fmuladd to FMA machine instructions in certain
> > > > situations.
> > > >
> > > > A simple lit test is as follows:
> > > >
> > > >
> > > > ```
> > > > ; RUN: %clang_cc1 -triple amdgcn -S -o - %s| FileCheck %s
> > > >
> > > > define amdgpu_kernel void @f(double addrspace(1)* nocapture %out,
> > > > double %a, double %b, double %c) local_unnamed_addr #0 {
> > > > entry:
> > > > ; CHECK: v_fma_f64
> > > > %0 = tail call double @llvm.fmuladd.f64(double %b, double %c, double
> > > > %a)
> > > > store double %0, double addrspace(1)* %out, align 8, !tbaa !6
> > > > ret void
> > > > }
> > > >
> > > > declare double @llvm.fmuladd.f64(double, double, double) #1
> > > >
> > > > attributes #0 = { nounwind
> > > > "correctly-rounded-divide-sqrt-fp-math"="false"
> > > > "disable-tail-calls"="false" "less-precise-fpmad"="false"
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment") yaxunl wrote: > anemet wrote: > > yaxunl wrote: > > > This change seemed to cause some performance degradations in some OpenCL > > > applications. > > > > > > This option used to be on by default. Now it is off by default. > > > > > > There are applications do separated compile/link/codegen stages. In the > > > codegen stage, clang is invoked with .bc as input, therefore this option > > > is off by default, whereas it was on by default before this change. > > > > > > Is there any reason not to keep the original behavior? > > > > > > Thanks. > > > This change seemed to cause some performance degradations in some OpenCL > > > applications. > > > > > > This option used to be on by default. Now it is off by default. > > > > It's always been off. It was set to fast for CUDA which should still be > > the case. See the changes further down on the patch. > > > > > > > > There are applications do separated compile/link/codegen stages. In the > > > codegen stage, clang is invoked with .bc as input, therefore this option > > > is off by default, whereas it was on by default before this change. > > > > > > Is there any reason not to keep the original behavior? > > > > Sorry but I am not sure what changed, can you elaborate on what you're > > doing and how things used to work for you? > Before your change, -ffp-contract was a codegen option defined as > > > ``` > ENUM_CODEGENOPT(FPContractMode, FPContractModeKind, 2, FPC_On) > ``` > > therefore the default value as on. After your change, it becomes off by > default. No. -ffp-contract=on was a FE option and -ffp-contract=fast was a backend option. I still don't understand your use-case. Can you include a small testcase how this used to work before? Repository: rL LLVM https://reviews.llvm.org/D31167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment") yaxunl wrote: > This change seemed to cause some performance degradations in some OpenCL > applications. > > This option used to be on by default. Now it is off by default. > > There are applications do separated compile/link/codegen stages. In the > codegen stage, clang is invoked with .bc as input, therefore this option is > off by default, whereas it was on by default before this change. > > Is there any reason not to keep the original behavior? > > Thanks. > This change seemed to cause some performance degradations in some OpenCL > applications. > > This option used to be on by default. Now it is off by default. It's always been off. It was set to fast for CUDA which should still be the case. See the changes further down on the patch. > > There are applications do separated compile/link/codegen stages. In the > codegen stage, clang is invoked with .bc as input, therefore this option is > off by default, whereas it was on by default before this change. > > Is there any reason not to keep the original behavior? Sorry but I am not sure what changed, can you elaborate on what you're doing and how things used to work for you? Repository: rL LLVM https://reviews.llvm.org/D31167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299470: Add #pragma clang fp (authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D31276?vs=93325=94122#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31276
Files:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/test/CodeGen/fp-contract-fast-pragma.cpp
cfe/trunk/test/CodeGen/fp-contract-on-pragma.cpp
cfe/trunk/test/Parser/cxx11-stmt-attributes.cpp
cfe/trunk/test/Parser/pragma-fp.cpp
Index: cfe/trunk/test/Parser/pragma-fp.cpp
===
--- cfe/trunk/test/Parser/pragma-fp.cpp
+++ cfe/trunk/test/Parser/pragma-fp.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+void test_0(int *List, int Length) {
+/* expected-error@+1 {{missing option; expected contract}} */
+#pragma clang fp
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+void test_1(int *List, int Length) {
+/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+#pragma clang fp blah
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_3(int *List, int Length) {
+/* expected-error@+1 {{expected '('}} */
+#pragma clang fp contract on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_4(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+#pragma clang fp contract(while)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_5(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+#pragma clang fp contract(maybe)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_6(int *List, int Length) {
+/* expected-error@+1 {{expected ')'}} */
+#pragma clang fp contract(fast
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_7(int *List, int Length) {
+/* expected-warning@+1 {{extra tokens at end of '#pragma clang fp' - ignored}} */
+#pragma clang fp contract(fast) *
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_8(int *List, int Length) {
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+/* expected-error@+1 {{'#pragma clang fp' can only appear at file scope or at the start of a compound statement}} */
+#pragma clang fp contract(fast)
+ }
+}
Index: cfe/trunk/test/Parser/cxx11-stmt-attributes.cpp
===
--- cfe/trunk/test/Parser/cxx11-stmt-attributes.cpp
+++ cfe/trunk/test/Parser/cxx11-stmt-attributes.cpp
@@ -80,5 +80,6 @@
{
[[ ]] // expected-error {{an attribute list cannot appear here}}
#pragma STDC FP_CONTRACT ON // expected-error {{can only appear at file scope or at the start of a compound statement}}
+#pragma clang fp contract(fast) // expected-error {{can only appear at file scope or at the start of a compound statement}}
}
}
Index: cfe/trunk/test/CodeGen/fp-contract-fast-pragma.cpp
===
--- cfe/trunk/test/CodeGen/fp-contract-fast-pragma.cpp
+++ cfe/trunk/test/CodeGen/fp-contract-fast-pragma.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+// Is FP_CONTRACT honored in a simple case?
+float fp_contract_1(float a, float b, float c) {
+// CHECK: _Z13fp_contract_1fff
+// CHECK: %[[M:.+]] = fmul contract float %a, %b
+// CHECK-NEXT: fadd contract float %[[M]], %c
+#pragma clang fp contract(fast)
+ return a * b + c;
+}
+
+// Is FP_CONTRACT state cleared on exiting compound statements?
+float fp_contract_2(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_2fff
+ // CHECK: %[[M:.+]] = fmul float %a, %b
+ // CHECK-NEXT: fadd float %[[M]], %c
+ {
+#pragma clang fp contract(fast)
+ }
+ return a * b + c;
+}
+
+// Does FP_CONTRACT survive template instantiation?
+class Foo {};
+Foo operator+(Foo, Foo);
+
+template
+T template_muladd(T a, T b, T c) {
+#pragma clang fp contract(fast)
+ return a * b + c;
+}
+
+float fp_contract_3(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_3fff
+ // CHECK: %[[M:.+]] = fmul contract float %a, %b
+ // CHECK-NEXT: fadd contract float %[[M]], %c
+ return template_muladd(a, b, c);
+}
+
+template
+class fp_contract_4 {
+ float method(float a, float b, float c) {
+#pragma clang fp contract(fast)
+return a * b + c;
+ }
+};
+
+template class fp_contract_4;
+// CHECK: _ZN13fp_contract_4IiE6methodEfff
+// CHECK: %[[M:.+]] = fmul
[PATCH] D31168: Set FMF for -ffp-contract=fast
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299469: Set FMF for -ffp-contract=fast (authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D31168?vs=93882=94121#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31168
Files:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
Index: cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
===
--- cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
+++ cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple
-emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -113,6 +113,22 @@
(2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
}
+/// Update the FastMathFlags of LLVM IR from the FPOptions in LangOptions.
+static void updateFastMathFlags(llvm::FastMathFlags ,
+FPOptions FPFeatures) {
+ FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement());
+}
+
+/// Propagate fast-math flags from \p Op to the instruction in \p V.
+static Value *propagateFMFlags(Value *V, const BinOpInfo ) {
+ if (auto *I = dyn_cast(V)) {
+llvm::FastMathFlags FMF = I->getFastMathFlags();
+updateFastMathFlags(FMF, Op.FPFeatures);
+I->setFastMathFlags(FMF);
+ }
+ return V;
+}
+
class ScalarExprEmitter
: public StmtVisitor {
CodeGenFunction
@@ -553,8 +569,10 @@
!CanElideOverflowCheck(CGF.getContext(), Ops))
return EmitOverflowCheckedBinOp(Ops);
-if (Ops.LHS->getType()->isFPOrFPVectorTy())
- return Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
+ Value *V = Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+ return propagateFMFlags(V, Ops);
+}
return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
}
/// Create a binary op that checks for overflow.
@@ -2722,7 +2740,8 @@
if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder))
return FMulAdd;
-return Builder.CreateFAdd(op.LHS, op.RHS, "add");
+Value *V = Builder.CreateFAdd(op.LHS, op.RHS, "add");
+return propagateFMFlags(V, op);
}
return Builder.CreateAdd(op.LHS, op.RHS, "add");
@@ -2755,7 +2774,8 @@
// Try to form an fmuladd.
if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder, true))
return FMulAdd;
- return Builder.CreateFSub(op.LHS, op.RHS, "sub");
+ Value *V = Builder.CreateFSub(op.LHS, op.RHS, "sub");
+ return propagateFMFlags(V, op);
}
return Builder.CreateSub(op.LHS, op.RHS, "sub");
Index: cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
===
--- cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
+++ cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -113,6 +113,22 @@
(2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
}
+/// Update the
[PATCH] D31168: Set FMF for -ffp-contract=fast
anemet added a comment. Thanks, Aaron! @rjmccall, does it look good to you too? https://reviews.llvm.org/D31168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fp
anemet added a comment. In https://reviews.llvm.org/D31276#717390, @aaron.ballman wrote: > This continues to look good to me with the new name. Thank you, Aaron! https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31168: Set FMF for -ffp-contract=fast
anemet updated this revision to Diff 93882.
anemet added a comment.
Address John's comment.
https://reviews.llvm.org/D31168
Files:
lib/CodeGen/CGExprScalar.cpp
test/CodeGen/ffp-contract-fast-option.cpp
Index: test/CodeGen/ffp-contract-fast-option.cpp
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple
-emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -113,6 +113,22 @@
(2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
}
+/// Update the FastMathFlags of LLVM IR from the FPOptions in LangOptions.
+static void updateFastMathFlags(llvm::FastMathFlags ,
+FPOptions FPFeatures) {
+ FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement());
+}
+
+/// Propagate fast-math flags from \p Op to the instruction in \p V.
+static Value *propagateFMFlags(Value *V, const BinOpInfo ) {
+ if (auto *I = dyn_cast(V)) {
+llvm::FastMathFlags FMF = I->getFastMathFlags();
+updateFastMathFlags(FMF, Op.FPFeatures);
+I->setFastMathFlags(FMF);
+ }
+ return V;
+}
+
class ScalarExprEmitter
: public StmtVisitor {
CodeGenFunction
@@ -553,8 +569,10 @@
!CanElideOverflowCheck(CGF.getContext(), Ops))
return EmitOverflowCheckedBinOp(Ops);
-if (Ops.LHS->getType()->isFPOrFPVectorTy())
- return Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
+ Value *V = Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+ return propagateFMFlags(V, Ops);
+}
return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
}
/// Create a binary op that checks for overflow.
@@ -2722,7 +2740,8 @@
if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder))
return FMulAdd;
-return Builder.CreateFAdd(op.LHS, op.RHS, "add");
+Value *V = Builder.CreateFAdd(op.LHS, op.RHS, "add");
+return propagateFMFlags(V, op);
}
return Builder.CreateAdd(op.LHS, op.RHS, "add");
@@ -2755,7 +2774,8 @@
// Try to form an fmuladd.
if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder, true))
return FMulAdd;
- return Builder.CreateFSub(op.LHS, op.RHS, "sub");
+ Value *V = Builder.CreateFSub(op.LHS, op.RHS, "sub");
+ return propagateFMFlags(V, op);
}
return Builder.CreateSub(op.LHS, op.RHS, "sub");
Index: test/CodeGen/ffp-contract-fast-option.cpp
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -113,6 +113,22 @@
(2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
}
+/// Update the FastMathFlags of LLVM IR from the FPOptions in LangOptions.
+static void updateFastMathFlags(llvm::FastMathFlags ,
+FPOptions FPFeatures) {
+ FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement());
+}
+
+/// Propagate fast-math flags from \p Op to the instruction in \p V.
+static Value *propagateFMFlags(Value *V, const
[PATCH] D31168: Set FMF for -ffp-contract=fast
anemet added a comment. In https://reviews.llvm.org/D31168#716153, @rjmccall wrote: > I may have missed earlier steps in this patch series. Why is this being done > statefully and contextually in the IRBuilder instead of just applying the > flag from the BinaryOperator to the instruction when building it? It's not > like ScalarExprEmitter doesn't know that it's building an FMul. The main reason is that the other FMFlags are currently maintained in the IRBuilder (see CodeGenFunction.cpp:87). That said as we move those over to the operators as well, it makes more sense to move away from using the IRBuilder for this. See updated patch and thanks for the suggestion! Also let me know if you have post-commit comments on the patches in the series. You can find them either on the cfe-dev thread or in the dependencies of https://reviews.llvm.org/D31276 https://reviews.llvm.org/D31168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31168: Set FMF for -ffp-contract=fast
anemet added a comment. Ping. This is the only patch in the series now that wasn't approved. https://reviews.llvm.org/D31168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31168: Set FMF for -ffp-contract=fast
anemet updated this revision to Diff 93487.
anemet added a comment.
Also add 'contract' for CompoundAssignOperator (+= and -=). For l-values,
these don't go through the expr-visitor in ScalarExprEmitter::Visit. This
again piggybacks on how debug-locations are added.
https://reviews.llvm.org/D31168
Files:
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGen/ffp-contract-fast-option.cpp
Index: test/CodeGen/ffp-contract-fast-option.cpp
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple
-emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3822,6 +3822,25 @@
}
};
+/// A RAII to apply the FP features from the expression to the IRBuilder.
+struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ static Optional getFPFeatures(const Expr *E) {
+if (const auto *BO = dyn_cast(E))
+ return BO->getFPFeatures();
+else if (const auto *CE = dyn_cast(E))
+ return CE->getFPFeatures();
+return None;
+ }
+
+ ApplyFPFeatures(llvm::IRBuilderBase , const Expr *E)
+ : CGBuilderTy::FastMathFlagGuard(Builder) {
+if (Optional FPFeatures = getFPFeatures(E)) {
+ llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+ FMF.setAllowContract(FPFeatures->allowFPContractAcrossStatement());
+ Builder.setFastMathFlags(FMF);
+}
+ }
+};
} // end namespace CodeGen
} // end namespace clang
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -258,6 +258,7 @@
Value *Visit(Expr *E) {
ApplyDebugLocation DL(CGF, E);
+ApplyFPFeatures FPF(Builder, E);
return StmtVisitor::Visit(E);
}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1013,6 +1013,7 @@
///
LValue CodeGenFunction::EmitLValue(const Expr *E) {
ApplyDebugLocation DL(*this, E);
+ ApplyFPFeatures FPF(Builder, E);
switch (E->getStmtClass()) {
default: return EmitUnsupportedLValue(E, "l-value expression");
Index: test/CodeGen/ffp-contract-fast-option.cpp
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_1fff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: fp_contract_2fff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_3Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+ // CHECK-LABEL: fp_contract_4Pfff(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ a[0] -= b * c;
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3822,6 +3822,25 @@
}
};
+/// A RAII to apply the FP features from the expression to the IRBuilder.
+struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ static Optional getFPFeatures(const Expr *E) {
+if (const auto *BO = dyn_cast(E))
+ return BO->getFPFeatures();
+else if (const auto *CE = dyn_cast(E))
+ return CE->getFPFeatures();
+return None;
+ }
+
+ ApplyFPFeatures(llvm::IRBuilderBase , const Expr *E)
+ : CGBuilderTy::FastMathFlagGuard(Builder) {
+if (Optional FPFeatures = getFPFeatures(E)) {
+ llvm::FastMathFlags FMF =
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217 /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this. - unsigned fp_contract : 1; + LangOptions::FPContractModeKind fp_contract : 2; }; rnk wrote: > Please do not use bitfields with enum types, it's a good way to break the > build on Windows. This change triggered this clang-cl warning: > ``` > C:\src\llvm-project\clang\include\clang/Basic/LangOptions.h(208,17): > warning: implicit truncation from 'clang::LangOptions::FPContractModeKind' to > bit-field changes value from 2 to -2 [-Wbitfield-constant-conversion] > fp_contract = LangOptions::FPC_Fast; > ^ ~ > ``` Noted and thanks for the fix! Unfortunately the warning wasn't showing up on my host. I'll take a look why. Repository: rL LLVM https://reviews.llvm.org/D31167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31167: Use FPContractModeKind universally
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299027: Use FPContractModeKind universally (authored by
anemet).
Changed prior to commit:
https://reviews.llvm.org/D31167?vs=92423=93406#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31167
Files:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
Index: cfe/trunk/lib/Sema/SemaAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaAttr.cpp
+++ cfe/trunk/lib/Sema/SemaAttr.cpp
@@ -450,13 +450,16 @@
void Sema::ActOnPragmaFPContract(tok::OnOffSwitch OOS) {
switch (OOS) {
case tok::OOS_ON:
-FPFeatures.setFPContractable(true);
+FPFeatures.setAllowFPContractWithinStatement();
break;
case tok::OOS_OFF:
-FPFeatures.setFPContractable(false);
+FPFeatures.setDisallowFPContract();
break;
case tok::OOS_DEFAULT:
-FPFeatures.setFPContractable(getLangOpts().DefaultFPContract);
+if (getLangOpts().getDefaultFPContractMode() == LangOptions::FPC_On)
+ FPFeatures.setAllowFPContractWithinStatement();
+else
+ FPFeatures.setDisallowFPContract();
break;
}
}
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -816,18 +816,6 @@
}
}
- if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
-StringRef Val = A->getValue();
-if (Val == "fast")
- Opts.setFPContractMode(CodeGenOptions::FPC_Fast);
-else if (Val == "on")
- Opts.setFPContractMode(CodeGenOptions::FPC_On);
-else if (Val == "off")
- Opts.setFPContractMode(CodeGenOptions::FPC_Off);
-else
- Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
- }
-
if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
StringRef Val = A->getValue();
if (Val == "ieee")
@@ -1647,7 +1635,7 @@
Opts.AltiVec = 0;
Opts.ZVector = 0;
Opts.LaxVectorConversions = 0;
-Opts.DefaultFPContract = 1;
+Opts.setDefaultFPContractMode(LangOptions::FPC_On);
Opts.NativeHalfType = 1;
Opts.NativeHalfArgsAndReturns = 1;
// Include default header file for OpenCL.
@@ -1658,6 +1646,9 @@
Opts.CUDA = IK == IK_CUDA || IK == IK_PreprocessedCuda ||
LangStd == LangStandard::lang_cuda;
+ if (Opts.CUDA)
+// Set default FP_CONTRACT to FAST.
+Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
Opts.RenderScript = IK == IK_RenderScript;
if (Opts.RenderScript) {
@@ -2282,6 +2273,18 @@
Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
Args.hasArg(OPT_cl_fast_relaxed_math);
+ if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
+StringRef Val = A->getValue();
+if (Val == "fast")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
+else if (Val == "on")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_On);
+else if (Val == "off")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_Off);
+else
+ Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
+ }
+
Opts.RetainCommentsFromSystemHeaders =
Args.hasArg(OPT_fretain_comments_from_system_headers);
@@ -2536,10 +2539,6 @@
// triple used for host compilation.
if (LangOpts.CUDAIsDevice)
Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple;
-
-// Set default FP_CONTRACT to FAST.
-if (!Args.hasArg(OPT_ffp_contract))
- Res.getCodeGenOpts().setFPContractMode(CodeGenOptions::FPC_Fast);
}
// FIXME: Override value name discarding when asan or msan is used because the
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -2672,12 +2672,7 @@
"Only fadd/fsub can be the root of an fmuladd.");
// Check whether this op is marked as fusable.
- if (!op.FPFeatures.isFPContractable())
-return nullptr;
-
- // Check whether -ffp-contract=on. (If -ffp-contract=off/fast, fusing is
- // either disabled, or handled entirely by the LLVM backend).
- if (CGF.CGM.getCodeGenOpts().getFPContractMode() != CodeGenOptions::FPC_On)
+ if (!op.FPFeatures.allowFPContractWithinStatement())
return nullptr;
// We have a potentially fusable op. Look for a mul on one of the operands.
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
[PATCH] D31167: Use FPContractModeKind universally
anemet added inline comments.
Comment at: include/clang/Basic/LangOptions.h:92
+ enum FPContractModeKind {
+FPC_Off,// Form fused FP ops only where result will not be
affected.
+FPC_On, // Form fused FP ops according to FP_CONTRACT rules.
aaron.ballman wrote:
> I think you mean effected rather than affected.
I think the verb is affect; effect is the noun. Quick grep confirms:
/org/llvm$ git grep affected | wc -l
109
/org/llvm$ git grep effected | wc -l
2
https://reviews.llvm.org/D31167
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fp
anemet added a comment. (Sorry about updating the title/description back and forth but arcanist is buggy) https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet updated this revision to Diff 93325.
anemet retitled this revision from "Add #pragma clang fp" to "Add #pragma clang
fast_math".
anemet edited the summary of this revision.
anemet added a comment.
Rename pragma from #pragma clang fast_math contract_fast(on/off) -> #pragma
clang fp contract(on/fast/off)
No other functional change.
https://reviews.llvm.org/D31276
Files:
docs/LanguageExtensions.rst
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Basic/TokenKinds.def
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParsePragma.cpp
lib/Parse/ParseStmt.cpp
lib/Parse/Parser.cpp
lib/Sema/SemaAttr.cpp
test/CodeGen/fp-contract-fast-pragma.cpp
test/CodeGen/fp-contract-on-pragma.cpp
test/Parser/cxx11-stmt-attributes.cpp
test/Parser/pragma-fp.cpp
Index: test/Parser/pragma-fp.cpp
===
--- /dev/null
+++ test/Parser/pragma-fp.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+void test_0(int *List, int Length) {
+/* expected-error@+1 {{missing option; expected contract}} */
+#pragma clang fp
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+void test_1(int *List, int Length) {
+/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+#pragma clang fp blah
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_3(int *List, int Length) {
+/* expected-error@+1 {{expected '('}} */
+#pragma clang fp contract on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_4(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+#pragma clang fp contract(while)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_5(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+#pragma clang fp contract(maybe)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_6(int *List, int Length) {
+/* expected-error@+1 {{expected ')'}} */
+#pragma clang fp contract(fast
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_7(int *List, int Length) {
+/* expected-warning@+1 {{extra tokens at end of '#pragma clang fp' - ignored}} */
+#pragma clang fp contract(fast) *
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_8(int *List, int Length) {
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+/* expected-error@+1 {{'#pragma clang fp' can only appear at file scope or at the start of a compound statement}} */
+#pragma clang fp contract(fast)
+ }
+}
Index: test/Parser/cxx11-stmt-attributes.cpp
===
--- test/Parser/cxx11-stmt-attributes.cpp
+++ test/Parser/cxx11-stmt-attributes.cpp
@@ -80,5 +80,6 @@
{
[[ ]] // expected-error {{an attribute list cannot appear here}}
#pragma STDC FP_CONTRACT ON // expected-error {{can only appear at file scope or at the start of a compound statement}}
+#pragma clang fp contract(fast) // expected-error {{can only appear at file scope or at the start of a compound statement}}
}
}
Index: test/CodeGen/fp-contract-on-pragma.cpp
===
--- /dev/null
+++ test/CodeGen/fp-contract-on-pragma.cpp
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+// Is FP_CONTRACT honored in a simple case?
+float fp_contract_1(float a, float b, float c) {
+// CHECK: _Z13fp_contract_1fff
+// CHECK: tail call float @llvm.fmuladd
+#pragma clang fp contract(on)
+ return a * b + c;
+}
+
+// Is FP_CONTRACT state cleared on exiting compound statements?
+float fp_contract_2(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_2fff
+ // CHECK: %[[M:.+]] = fmul float %a, %b
+ // CHECK-NEXT: fadd float %[[M]], %c
+ {
+#pragma clang fp contract(on)
+ }
+ return a * b + c;
+}
+
+// Does FP_CONTRACT survive template instantiation?
+class Foo {};
+Foo operator+(Foo, Foo);
+
+template
+T template_muladd(T a, T b, T c) {
+#pragma clang fp contract(on)
+ return a * b + c;
+}
+
+float fp_contract_3(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_3fff
+ // CHECK: tail call float @llvm.fmuladd
+ return template_muladd(a, b, c);
+}
+
+template
+class fp_contract_4 {
+ float method(float a, float b, float c) {
+#pragma clang fp contract(on)
+return a * b + c;
+ }
+};
+
+template class fp_contract_4;
+// CHECK: _ZN13fp_contract_4IiE6methodEfff
+// CHECK: tail call float @llvm.fmuladd
+
+// Check file-scoped FP_CONTRACT
+#pragma clang fp contract(on)
+float fp_contract_5(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_5fff
+ // CHECK: tail call float @llvm.fmuladd
+ return a * b + c;
+}
+
+#pragma clang fp contract(off)
+float fp_contract_6(float a,
[PATCH] D31166: Encapsulate FPOptions and use it consistently
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298877: Encapsulate FPOptions and use it consistently
(authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D31166?vs=92898=93166#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31166
Files:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -7500,7 +7500,7 @@
BinaryOperator *addExpr =
new (Context) BinaryOperator(castExpr, DRE, BO_Add,
Context->getPointerType(Context->CharTy),
- VK_RValue, OK_Ordinary, SourceLocation(), false);
+ VK_RValue, OK_Ordinary, SourceLocation(), FPOptions());
// Don't forget the parens to enforce the proper binding.
ParenExpr *PE = new (Context) ParenExpr(SourceLocation(),
SourceLocation(),
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
@@ -2992,7 +2992,7 @@
BinaryOperator *lessThanExpr =
new (Context) BinaryOperator(sizeofExpr, limit, BO_LE, Context->IntTy,
VK_RValue, OK_Ordinary, SourceLocation(),
- false);
+ FPOptions());
// (sizeof(returnType) <= 8 ? objc_msgSend(...) : objc_msgSend_stret(...))
ConditionalOperator *CondExpr =
new (Context) ConditionalOperator(lessThanExpr,
Index: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
@@ -670,7 +670,7 @@
E->setRHS(Record.readSubExpr());
E->setOpcode((BinaryOperator::Opcode)Record.readInt());
E->setOperatorLoc(ReadSourceLocation());
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
@@ -1225,7 +1225,7 @@
VisitCallExpr(E);
E->Operator = (OverloadedOperatorKind)Record.readInt();
E->Range = Record.readSourceRange();
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) {
Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -7378,7 +7378,7 @@
// FIXME: What happens if these are changed by a module import?
if (!FPPragmaOptions.empty()) {
assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS");
-SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0];
+SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]);
}
SemaObj->OpenCLFeatures.copy(OpenCLExtensions);
Index: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
+++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
@@ -650,7 +650,7 @@
Record.AddStmt(E->getRHS());
Record.push_back(E->getOpcode()); // FIXME: stable encoding
Record.AddSourceLocation(E->getOperatorLoc());
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_BINARY_OPERATOR;
}
@@ -1218,7 +1218,7 @@
VisitCallExpr(E);
Record.push_back(E->getOperator());
Record.AddSourceRange(E->Range);
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_CXX_OPERATOR_CALL;
}
[PATCH] D31276: Add #pragma clang fast_math
anemet added a comment. In https://reviews.llvm.org/D31276#710608, @hfinkel wrote: > In https://reviews.llvm.org/D31276#709111, @anemet wrote: > > > In https://reviews.llvm.org/D31276#708992, @hfinkel wrote: > > > > > High-level comment ;) > > > > > > #pragma clang fast_math contract_fast(on) > > > > > > > > > This seems a bit unfortunate because 'fast' appears twice? How are we > > > planning on naming the other fast-math flags? Maybe we should just name > > > it: > > > > > > This is just pure laziness on my part, I was hoping that all these flags > > can be implemented with on/off but if you find it confusing that's a good > > indicator. > > > > > > > > > > > #pragma clang math constract_fast(on) > > > > > > > > > or > > > > > > #pragma clang math contract(fast) // we could also accept off/on here > > > for consistency and compatibility with the standard pragma > > > > > > > > > or maybe fp_math or floating_point_math or floating_point or fp instead > > > of math. > > > > > > I think that I prefer this last form (because it does not repeat 'fast' > > > and also makes our extension a pure superset of the standard pragma). > > > > > > What do you want to name the other flags? I'd prefer if they're > > > grammatically consistent. Maybe we should stick closely to the > > > command-line options, and have: > > > > > > fp_contract(on/off/fast) > > > unsafe_optimizations(on/off) > > > finite_only(on/off) > > > > > > > > > What do you think? > > > > I really like #pragma clang fp or fp_math because contraction feels > > different from the other fast-math flags. That said then we don't want to > > repeat fp in fp_contract. > > > > We should probably have the full list to make sure it works though with all > > the FMFs. Here is a straw-man proposal: > > > > UnsafeAlgebra #pragma clang fp unsafe_optimizations(on/off) > > NoNaNs #pragma clang fp no_nans(on/off) > > NoInfs#pragma clang fp finite_only(on/off) > > NoSignedZeros #pragma clang fp no_signed_zeros(on/off) > > AllowReciprocal#pragma clang fp reciprocal_math > > AllowContract #pragma clang fp contract(on/off/fast) > > > > > > The negative ones feel a bit strange... What do you think? > > > I agree. The negative ones feel a bit strange. Why should we have no_nans(on) > instead of nans(off)? However, I feel that the negative sense is less > ambiguous - they better match how I think about it and makes a strict > environment one where all of these are 'off', and I like that consistency. In > short, I like this proposal. Great, thanks! I'll update the patch tonight. https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet updated this revision to Diff 92898.
anemet added a comment.
Address Aaron's comments.
https://reviews.llvm.org/D31166
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/Basic/LangOptions.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/Analysis/BodyFarm.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CGStmtOpenMP.cpp
lib/Frontend/Rewrite/RewriteModernObjC.cpp
lib/Frontend/Rewrite/RewriteObjC.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaPseudoObject.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/ASTWriterStmt.cpp
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -650,7 +650,7 @@
Record.AddStmt(E->getRHS());
Record.push_back(E->getOpcode()); // FIXME: stable encoding
Record.AddSourceLocation(E->getOperatorLoc());
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_BINARY_OPERATOR;
}
@@ -1218,7 +1218,7 @@
VisitCallExpr(E);
Record.push_back(E->getOperator());
Record.AddSourceRange(E->Range);
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_CXX_OPERATOR_CALL;
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4013,7 +4013,7 @@
/// \brief Write an FP_PRAGMA_OPTIONS block for the given FPOptions.
void ASTWriter::WriteFPPragmaOptions(const FPOptions ) {
- RecordData::value_type Record[] = {Opts.fp_contract};
+ RecordData::value_type Record[] = {Opts.getInt()};
Stream.EmitRecord(FP_PRAGMA_OPTIONS, Record);
}
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -670,7 +670,7 @@
E->setRHS(Record.readSubExpr());
E->setOpcode((BinaryOperator::Opcode)Record.readInt());
E->setOperatorLoc(ReadSourceLocation());
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
@@ -1225,7 +1225,7 @@
VisitCallExpr(E);
E->Operator = (OverloadedOperatorKind)Record.readInt();
E->Range = Record.readSourceRange();
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) {
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -7367,7 +7367,7 @@
// FIXME: What happens if these are changed by a module import?
if (!FPPragmaOptions.empty()) {
assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS");
-SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0];
+SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]);
}
SemaObj->OpenCLFeatures.copy(OpenCLExtensions);
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -9146,7 +9146,7 @@
return E;
Sema::FPContractStateRAII FPContractState(getSema());
- getSema().FPFeatures.fp_contract = E->isFPContractable();
+ getSema().FPFeatures = E->getFPFeatures();
return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(),
LHS.get(), RHS.get());
@@ -9626,7 +9626,7 @@
return SemaRef.MaybeBindToTemporary(E);
Sema::FPContractStateRAII FPContractState(getSema());
- getSema().FPFeatures.fp_contract = E->isFPContractable();
+ getSema().FPFeatures = E->getFPFeatures();
return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
E->getOperatorLoc(),
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -447,7 +447,8 @@
syntactic = new (S.Context) BinaryOperator(syntacticLHS, capturedRHS,
opcode, capturedRHS->getType(),
capturedRHS->getValueKind(),
- OK_Ordinary, opcLoc, false);
+ OK_Ordinary, opcLoc,
+
[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet marked 3 inline comments as done. anemet added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1712 BinOp.Opcode = IsInc ? BO_Add : BO_Sub; - BinOp.FPContractable = false; + // FIXME: once UnaryOperator carries FPFeatures, copy it here. BinOp.E = E; aaron.ballman wrote: > Why not make UnaryOperator carry this information now, since it's needed? The trouble is that currently it's not needed. I'd rather wait for a fast-math flag that actually needs it so that we can write tests. https://reviews.llvm.org/D31166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet added a comment. In https://reviews.llvm.org/D31276#708999, @hfinkel wrote: > They're definitely on my list. I might not get to them until the weekend or > next week, however. Thank you. The schedule is getting tight but that should still work ;). Actually most of the individual patches are pretty small both in clang and llvm. This one was the one that I felt the least confident about :). https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet added a comment. In https://reviews.llvm.org/D31276#708992, @hfinkel wrote: > High-level comment ;) > > #pragma clang fast_math contract_fast(on) > > > This seems a bit unfortunate because 'fast' appears twice? How are we > planning on naming the other fast-math flags? Maybe we should just name it: This is just pure laziness on my part, I was hoping that all these flags can be implemented with on/off but if you find it confusing that's a good indicator. > > > #pragma clang math constract_fast(on) > > > or > > #pragma clang math contract(fast) // we could also accept off/on here for > consistency and compatibility with the standard pragma > > > or maybe fp_math or floating_point_math or floating_point or fp instead of > math. > > I think that I prefer this last form (because it does not repeat 'fast' and > also makes our extension a pure superset of the standard pragma). > > What do you want to name the other flags? I'd prefer if they're grammatically > consistent. Maybe we should stick closely to the command-line options, and > have: > > fp_contract(on/off/fast) > unsafe_optimizations(on/off) > finite_only(on/off) > > > What do you think? I really like #pragma clang fp or fp_math because contraction feels different from the other fast-math flags. That said then we don't want to repeat fp in fp_contract. We should probably have the full list to make sure it works though with all the FMFs. Here is a straw-man proposal: UnsafeAlgebra #pragma clang fp unsafe_optimizations(on/off) NoNaNs #pragma clang fp no_nans(on/off) NoInfs#pragma clang fp finite_only(on/off) NoSignedZeros #pragma clang fp no_signed_zeros(on/off) AllowReciprocal#pragma clang fp reciprocal_math AllowContract #pragma clang fp contract(on/off/fast) The negative ones feel a bit strange... What do you think? https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet added a comment. In https://reviews.llvm.org/D31276#708976, @aaron.ballman wrote: > In https://reviews.llvm.org/D31276#708968, @anemet wrote: > > > Thanks very much, Aaron! It would be great if you could also look at the > > three patches that add support for the generation of the new FMF 'contract' > > for -ffp-contract=fast. I've added them in the dependencies of this > > revision. (https://reviews.llvm.org/D31168 is not really a prerequisite > > but I am planning to commit the patches in this order.) > > > I'll take a look, but feel less qualified to give a LGTM to those. NP, thanks for your help on this one. Hopefully @hfinkel or @mehdi_amini can take a look. https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet added a comment. Thanks very much, Aaron! It would be great if you could also look at the three patches that add support for the generation of the new FMF 'contract' for -ffp-contract=fast. I've added them in the dependencies of this revision. (https://reviews.llvm.org/D31168 is not really a prerequisite but I am planning to commit the patches in this order.) https://reviews.llvm.org/D31276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet added inline comments.
Comment at: docs/LanguageExtensions.rst:2321
+specified for a section of the source code. This pragma can only appear at
+file scope or at the start of a compound statement. When using within a
+compound statement, the pragma is active within the scope of the compound
aaron.ballman wrote:
> By "start of a compound statement", does that include or exclude things like
> comments? e.g., is this valid?
> ```
> {
> // Disable clang's fast-math
> #pragma clang fast_math contract_fast(off)
> }
> ```
Excluding comments. Added the words.
FTR, there is also test coverage for this already in the patch.
https://reviews.llvm.org/D31276
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31276: Add #pragma clang fast_math
anemet updated this revision to Diff 92825.
anemet marked 4 inline comments as done.
anemet added a comment.
Address Aaron's comments. Also add a code example to the documentation.
https://reviews.llvm.org/D31276
Files:
docs/LanguageExtensions.rst
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Basic/TokenKinds.def
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParsePragma.cpp
lib/Parse/ParseStmt.cpp
lib/Parse/Parser.cpp
lib/Sema/SemaAttr.cpp
test/CodeGen/fp-contract-fast-pragma.cpp
test/Parser/cxx11-stmt-attributes.cpp
test/Parser/pragma-fast-math.cpp
Index: test/Parser/pragma-fast-math.cpp
===
--- /dev/null
+++ test/Parser/pragma-fast-math.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+void test_0(int *List, int Length) {
+/* expected-error@+1 {{missing option; expected contract_fast}} */
+#pragma clang fast_math
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+void test_1(int *List, int Length) {
+/* expected-error@+1 {{invalid option 'blah'; expected contract_fast}} */
+#pragma clang fast_math blah
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_3(int *List, int Length) {
+/* expected-error@+1 {{expected '('}} */
+#pragma clang fast_math contract_fast on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_4(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fast_math contract_fast'; expected 'on' or 'off'}} */
+#pragma clang fast_math contract_fast(while)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_5(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fast_math contract_fast'; expected 'on' or 'off'}} */
+#pragma clang fast_math contract_fast(maybe)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_6(int *List, int Length) {
+/* expected-error@+1 {{expected ')'}} */
+#pragma clang fast_math contract_fast(on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_7(int *List, int Length) {
+/* expected-warning@+1 {{extra tokens at end of '#pragma clang fast_math' - ignored}} */
+#pragma clang fast_math contract_fast(on) *
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_8(int *List, int Length) {
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+/* expected-error@+1 {{'#pragma clang fast_math' can only appear at file scope or at the start of a compound statement}} */
+#pragma clang fast_math contract_fast(on)
+ }
+}
Index: test/Parser/cxx11-stmt-attributes.cpp
===
--- test/Parser/cxx11-stmt-attributes.cpp
+++ test/Parser/cxx11-stmt-attributes.cpp
@@ -80,5 +80,6 @@
{
[[ ]] // expected-error {{an attribute list cannot appear here}}
#pragma STDC FP_CONTRACT ON // expected-error {{can only appear at file scope or at the start of a compound statement}}
+#pragma clang fast_math contract_fast(on) // expected-error {{can only appear at file scope or at the start of a compound statement}}
}
}
Index: test/CodeGen/fp-contract-fast-pragma.cpp
===
--- /dev/null
+++ test/CodeGen/fp-contract-fast-pragma.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+// Is FP_CONTRACT honored in a simple case?
+float fp_contract_1(float a, float b, float c) {
+// CHECK: _Z13fp_contract_1fff
+// CHECK: %[[M:.+]] = fmul contract float %a, %b
+// CHECK-NEXT: fadd contract float %[[M]], %c
+#pragma clang fast_math contract_fast(on)
+ return a * b + c;
+}
+
+// Is FP_CONTRACT state cleared on exiting compound statements?
+float fp_contract_2(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_2fff
+ // CHECK: %[[M:.+]] = fmul float %a, %b
+ // CHECK-NEXT: fadd float %[[M]], %c
+ {
+#pragma clang fast_math contract_fast(on)
+ }
+ return a * b + c;
+}
+
+// Does FP_CONTRACT survive template instantiation?
+class Foo {};
+Foo operator+(Foo, Foo);
+
+template
+T template_muladd(T a, T b, T c) {
+#pragma clang fast_math contract_fast(on)
+ return a * b + c;
+}
+
+float fp_contract_3(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_3fff
+ // CHECK: %[[M:.+]] = fmul contract float %a, %b
+ // CHECK-NEXT: fadd contract float %[[M]], %c
+ return template_muladd(a, b, c);
+}
+
+template
+class fp_contract_4 {
+ float method(float a, float b, float c) {
+#pragma clang fast_math contract_fast(on)
+return a * b + c;
+ }
+};
+
+template class fp_contract_4;
+// CHECK: _ZN13fp_contract_4IiE6methodEfff
+// CHECK: %[[M:.+]] = fmul contract float %a, %b
+// CHECK-NEXT: fadd contract float %[[M]], %c
+
+// Check file-scoped FP_CONTRACT
+#pragma clang fast_math contract_fast(on)
+float fp_contract_5(float a, float b,
[PATCH] D31276: Add #pragma clang fast_math
anemet created this revision.
This adds the new pragma and the first variant, contract_fast.
The pragma has the same block scope rules as STDC FP_CONTRACT, i.e. it can be
placed at the beginning of a compound statement or at file scope.
Similarly to STDC FP_CONTRACT there is no need to use attributes. First an
annotate token is inserted with the parsed details of the pragma. Then the
annotate token is parsed in the proper contexts and the Sema is updated with
the corresponding FPOptions using the shared ActOn function with STDC
FP_CONTRACT.
After this the FPOptions from the Sema is propagated into the AST expression
nodes. There is no change here.
I was going to add a 'default' option besides 'on' and 'off' similar to STDC
FP_CONTRACT but then decided against it. I think that we'd have to make option
uppercase then to avoid using 'default' the keyword. Also because of the
scoped activation of pragma I am not sure there is really a need a for this.
https://reviews.llvm.org/D31276
Files:
docs/LanguageExtensions.rst
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Basic/TokenKinds.def
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParsePragma.cpp
lib/Parse/ParseStmt.cpp
lib/Parse/Parser.cpp
lib/Sema/SemaAttr.cpp
test/CodeGen/fp-contract-fast-pragma.cpp
test/Parser/cxx11-stmt-attributes.cpp
test/Parser/pragma-fast-math.cpp
Index: test/Parser/pragma-fast-math.cpp
===
--- /dev/null
+++ test/Parser/pragma-fast-math.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+void test_0(int *List, int Length) {
+/* expected-error@+1 {{missing option; expected contract_fast}} */
+#pragma clang fast_math
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+void test_1(int *List, int Length) {
+/* expected-error@+1 {{invalid option 'blah'; expected contract_fast}} */
+#pragma clang fast_math blah
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_3(int *List, int Length) {
+/* expected-error@+1 {{expected '('}} */
+#pragma clang fast_math contract_fast on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_4(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fast_math contract_fast'; expected 'on' or 'off'}} */
+#pragma clang fast_math contract_fast(while)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_5(int *List, int Length) {
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fast_math contract_fast'; expected 'on' or 'off'}} */
+#pragma clang fast_math contract_fast(maybe)
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_6(int *List, int Length) {
+/* expected-error@+1 {{expected ')'}} */
+#pragma clang fast_math contract_fast(on
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_7(int *List, int Length) {
+/* expected-warning@+1 {{extra tokens at end of '#pragma clang fast_math' - ignored}} */
+#pragma clang fast_math contract_fast(on) *
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+ }
+}
+
+void test_8(int *List, int Length) {
+ for (int i = 0; i < Length; i++) {
+List[i] = i;
+/* expected-error@+1 {{'#pragma clang fast_math' can only appear at file scope or at the start of a compound statement}} */
+#pragma clang fast_math contract_fast(on)
+ }
+}
Index: test/Parser/cxx11-stmt-attributes.cpp
===
--- test/Parser/cxx11-stmt-attributes.cpp
+++ test/Parser/cxx11-stmt-attributes.cpp
@@ -80,5 +80,6 @@
{
[[ ]] // expected-error {{an attribute list cannot appear here}}
#pragma STDC FP_CONTRACT ON // expected-error {{can only appear at file scope or at the start of a compound statement}}
+#pragma clang fast_math contract_fast(on) // expected-error {{can only appear at file scope or at the start of a compound statement}}
}
}
Index: test/CodeGen/fp-contract-fast-pragma.cpp
===
--- /dev/null
+++ test/CodeGen/fp-contract-fast-pragma.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+// Is FP_CONTRACT honored in a simple case?
+float fp_contract_1(float a, float b, float c) {
+// CHECK: _Z13fp_contract_1fff
+// CHECK: %[[M:.+]] = fmul contract float %a, %b
+// CHECK-NEXT: fadd contract float %[[M]], %c
+#pragma clang fast_math contract_fast(on)
+ return a * b + c;
+}
+
+// Is FP_CONTRACT state cleared on exiting compound statements?
+float fp_contract_2(float a, float b, float c) {
+ // CHECK: _Z13fp_contract_2fff
+ // CHECK: %[[M:.+]] = fmul float %a, %b
+ // CHECK-NEXT: fadd float %[[M]], %c
+ {
+#pragma clang fast_math contract_fast(on)
+ }
+ return a * b + c;
+}
+
+// Does FP_CONTRACT survive template instantiation?
+class Foo {};
+Foo operator+(Foo, Foo);
+
[PATCH] D31168: Set FMF for -ffp-contract=fast rather than a TargetConfig
anemet updated this revision to Diff 92564.
anemet added a comment.
Address Akira's comments
https://reviews.llvm.org/D31168
Files:
lib/CodeGen/CGExprScalar.cpp
test/CodeGen/ffp-contract-fast-option.c
Index: test/CodeGen/ffp-contract-fast-option.c
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple
-emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: @fp_contract_1(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: @fp_contract_2(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -252,12 +252,33 @@
return Builder.CreateIsNotNull(V, "tobool");
}
+ static Optional getFPFeatures(Expr *E) {
+if (const auto *BO = dyn_cast(E))
+ return BO->getFPFeatures();
+else if (const auto *CE = dyn_cast(E))
+ return CE->getFPFeatures();
+return None;
+ }
+
+ /// A RAII to apply the FP features from the expression to the IRBuilder.
+ struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ApplyFPFeatures(llvm::IRBuilderBase , Expr *E)
+: CGBuilderTy::FastMathFlagGuard(Builder) {
+ if (Optional FPFeatures = getFPFeatures(E)) {
+llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+FMF.setAllowContract(FPFeatures->allowFPContractAcrossStatement());
+Builder.setFastMathFlags(FMF);
+ }
+}
+ };
+
//======//
//Visitor Methods
//======//
Value *Visit(Expr *E) {
ApplyDebugLocation DL(CGF, E);
+ApplyFPFeatures FPF(Builder, E);
return StmtVisitor::Visit(E);
}
Index: test/CodeGen/ffp-contract-fast-option.c
===
--- /dev/null
+++ test/CodeGen/ffp-contract-fast-option.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+ // CHECK-LABEL: @fp_contract_1(
+ // CHECK: fmul contract float
+ // CHECK: fadd contract float
+ return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+ // CHECK-LABEL: @fp_contract_2(
+ // CHECK: fmul contract float
+ // CHECK: fsub contract float
+ return a * b - c;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -252,12 +252,33 @@
return Builder.CreateIsNotNull(V, "tobool");
}
+ static Optional getFPFeatures(Expr *E) {
+if (const auto *BO = dyn_cast(E))
+ return BO->getFPFeatures();
+else if (const auto *CE = dyn_cast(E))
+ return CE->getFPFeatures();
+return None;
+ }
+
+ /// A RAII to apply the FP features from the expression to the IRBuilder.
+ struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ApplyFPFeatures(llvm::IRBuilderBase , Expr *E)
+: CGBuilderTy::FastMathFlagGuard(Builder) {
+ if (Optional FPFeatures = getFPFeatures(E)) {
+llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+FMF.setAllowContract(FPFeatures->allowFPContractAcrossStatement());
+Builder.setFastMathFlags(FMF);
+ }
+}
+ };
+
//======//
//Visitor Methods
//======//
Value *Visit(Expr *E) {
ApplyDebugLocation DL(CGF, E);
+ApplyFPFeatures FPF(Builder, E);
return StmtVisitor::Visit(E);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31168: Set FMF for -ffp-contract=fast rather than a TargetConfig
anemet added a comment. Thanks, Akira. You're right on both accounts. I actually had a test written but failed to git add it. https://reviews.llvm.org/D31168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31168: Set FMF for -ffp-contract=fast rather than a TargetConfig
anemet created this revision.
This is toward moving fp-contraction=fast from an LLVM TargetOption to a
FastMathFlag in order to fix PR25721.
https://reviews.llvm.org/D31168
Files:
lib/CodeGen/CGExprScalar.cpp
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -256,8 +256,22 @@
//Visitor Methods
//======//
+ /// A RAII to apply the FP features from the expression to the IRBuilder.
+ struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ApplyFPFeatures(llvm::IRBuilderBase , Expr *E)
+: CGBuilderTy::FastMathFlagGuard(Builder) {
+ if (const auto *BO = dyn_cast(E)) {
+llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+FMF.setAllowContract(
+BO->getFPFeatures().allowFPContractAcrossStatement());
+Builder.setFastMathFlags(FMF);
+ }
+}
+ };
+
Value *Visit(Expr *E) {
ApplyDebugLocation DL(CGF, E);
+ApplyFPFeatures FPF(Builder, E);
return StmtVisitor::Visit(E);
}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -256,8 +256,22 @@
//Visitor Methods
//======//
+ /// A RAII to apply the FP features from the expression to the IRBuilder.
+ struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+ApplyFPFeatures(llvm::IRBuilderBase , Expr *E)
+: CGBuilderTy::FastMathFlagGuard(Builder) {
+ if (const auto *BO = dyn_cast(E)) {
+llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+FMF.setAllowContract(
+BO->getFPFeatures().allowFPContractAcrossStatement());
+Builder.setFastMathFlags(FMF);
+ }
+}
+ };
+
Value *Visit(Expr *E) {
ApplyDebugLocation DL(CGF, E);
+ApplyFPFeatures FPF(Builder, E);
return StmtVisitor::Visit(E);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31167: Use FPContractModeKind universally
anemet created this revision.
FPContractModeKind is the codegen option flag which is already ternary (off,
on, fast). This makes it universally the type for the contractable info
across the front-end:
- In FPOptions (i.e. in the Sema + in the expression nodes).
- In LangOpts::DefaultFPContractMode which is the option that initializes
FPOptions in the Sema.
Another way to look at this change is that before fp-contractable on/off were
the only states handled to the front-end:
- For "on", FMA folding was performed by the front-end
- For "fast", we simply forwarded the flag to TargetOptions to handle it in LLVM
Now off/on/fast are all exposed because for fast we will generate
fast-math-flags during CodeGen.
This is toward moving fp-contraction=fast from an LLVM TargetOption to a
FastMathFlag in order to fix PR25721.
https://reviews.llvm.org/D31167
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/Basic/LangOptions.def
include/clang/Basic/LangOptions.h
include/clang/Frontend/CodeGenOptions.def
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Sema/SemaAttr.cpp
Index: lib/Sema/SemaAttr.cpp
===
--- lib/Sema/SemaAttr.cpp
+++ lib/Sema/SemaAttr.cpp
@@ -450,13 +450,16 @@
void Sema::ActOnPragmaFPContract(tok::OnOffSwitch OOS) {
switch (OOS) {
case tok::OOS_ON:
-FPFeatures.setFPContractable(true);
+FPFeatures.setAllowFPContractWithinStatement();
break;
case tok::OOS_OFF:
-FPFeatures.setFPContractable(false);
+FPFeatures.setDisallowFPContract();
break;
case tok::OOS_DEFAULT:
-FPFeatures.setFPContractable(getLangOpts().DefaultFPContract);
+if (getLangOpts().getDefaultFPContractMode() == LangOptions::FPC_On)
+ FPFeatures.setAllowFPContractWithinStatement();
+else
+ FPFeatures.setDisallowFPContract();
break;
}
}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -811,18 +811,6 @@
}
}
- if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
-StringRef Val = A->getValue();
-if (Val == "fast")
- Opts.setFPContractMode(CodeGenOptions::FPC_Fast);
-else if (Val == "on")
- Opts.setFPContractMode(CodeGenOptions::FPC_On);
-else if (Val == "off")
- Opts.setFPContractMode(CodeGenOptions::FPC_Off);
-else
- Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
- }
-
if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
StringRef Val = A->getValue();
if (Val == "ieee")
@@ -1625,7 +1613,7 @@
Opts.ZVector = 0;
Opts.CXXOperatorNames = 1;
Opts.LaxVectorConversions = 0;
-Opts.DefaultFPContract = 1;
+Opts.setDefaultFPContractMode(LangOptions::FPC_On);
Opts.NativeHalfType = 1;
Opts.NativeHalfArgsAndReturns = 1;
// Include default header file for OpenCL.
@@ -1636,6 +1624,9 @@
Opts.CUDA = IK == IK_CUDA || IK == IK_PreprocessedCuda ||
LangStd == LangStandard::lang_cuda;
+ if (Opts.CUDA)
+// Set default FP_CONTRACT to FAST.
+Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
Opts.RenderScript = IK == IK_RenderScript;
if (Opts.RenderScript) {
@@ -2263,6 +2254,18 @@
Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
Args.hasArg(OPT_cl_fast_relaxed_math);
+ if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
+StringRef Val = A->getValue();
+if (Val == "fast")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
+else if (Val == "on")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_On);
+else if (Val == "off")
+ Opts.setDefaultFPContractMode(LangOptions::FPC_Off);
+else
+ Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
+ }
+
Opts.RetainCommentsFromSystemHeaders =
Args.hasArg(OPT_fretain_comments_from_system_headers);
@@ -2516,10 +2519,6 @@
// triple used for host compilation.
if (LangOpts.CUDAIsDevice)
Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple;
-
-// Set default FP_CONTRACT to FAST.
-if (!Args.hasArg(OPT_ffp_contract))
- Res.getCodeGenOpts().setFPContractMode(CodeGenOptions::FPC_Fast);
}
// FIXME: Override value name discarding when asan or msan is used because the
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2663,12 +2663,7 @@
"Only fadd/fsub can be the root of an fmuladd.");
// Check whether this op is marked as fusable.
- if (!op.FPFeatures.isFPContractable())
-return nullptr;
-
- // Check whether -ffp-contract=on. (If -ffp-contract=off/fast,
[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet created this revision.
Sema holds the current FPOptions which is adjusted by 'pragma STDC
FP_CONTRACT'. This then gets propagated into expression nodes as they are
built.
This encapsulates FPOptions so that this propagation happens opaquely rather
than directly with the fp_contractable on/off bit. This allows controlled
transitioning of fp_contractable to a ternary value (off, on, fast). It will
also allow adding more fast-math flags later.
This is toward moving fp-contraction=fast from an LLVM TargetOption to a
FastMathFlag in order to fix PR25721.
https://reviews.llvm.org/D31166
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/Basic/LangOptions.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/Analysis/BodyFarm.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CGStmtOpenMP.cpp
lib/Frontend/Rewrite/RewriteModernObjC.cpp
lib/Frontend/Rewrite/RewriteObjC.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaPseudoObject.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/ASTWriterStmt.cpp
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -650,7 +650,7 @@
Record.AddStmt(E->getRHS());
Record.push_back(E->getOpcode()); // FIXME: stable encoding
Record.AddSourceLocation(E->getOperatorLoc());
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_BINARY_OPERATOR;
}
@@ -1218,7 +1218,7 @@
VisitCallExpr(E);
Record.push_back(E->getOperator());
Record.AddSourceRange(E->Range);
- Record.push_back(E->isFPContractable());
+ Record.push_back(E->getFPFeatures().getInt());
Code = serialization::EXPR_CXX_OPERATOR_CALL;
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3975,7 +3975,7 @@
/// \brief Write an FP_PRAGMA_OPTIONS block for the given FPOptions.
void ASTWriter::WriteFPPragmaOptions(const FPOptions ) {
- RecordData::value_type Record[] = {Opts.fp_contract};
+ RecordData::value_type Record[] = {Opts.getInt()};
Stream.EmitRecord(FP_PRAGMA_OPTIONS, Record);
}
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -670,7 +670,7 @@
E->setRHS(Record.readSubExpr());
E->setOpcode((BinaryOperator::Opcode)Record.readInt());
E->setOperatorLoc(ReadSourceLocation());
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
@@ -1225,7 +1225,7 @@
VisitCallExpr(E);
E->Operator = (OverloadedOperatorKind)Record.readInt();
E->Range = Record.readSourceRange();
- E->setFPContractable((bool)Record.readInt());
+ E->setFPFeatures(FPOptions(Record.readInt()));
}
void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) {
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -7215,7 +7215,7 @@
// FIXME: What happens if these are changed by a module import?
if (!FPPragmaOptions.empty()) {
assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS");
-SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0];
+SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]);
}
SemaObj->OpenCLFeatures.copy(OpenCLExtensions);
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -9146,7 +9146,7 @@
return E;
Sema::FPContractStateRAII FPContractState(getSema());
- getSema().FPFeatures.fp_contract = E->isFPContractable();
+ getSema().FPFeatures = E->getFPFeatures();
return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(),
LHS.get(), RHS.get());
@@ -9626,7 +9626,7 @@
return SemaRef.MaybeBindToTemporary(E);
Sema::FPContractStateRAII FPContractState(getSema());
- getSema().FPFeatures.fp_contract = E->isFPContractable();
+ getSema().FPFeatures = E->getFPFeatures();
return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
E->getOperatorLoc(),
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++
[PATCH] D27332: With LTO and profile-use, enable hotness info in opt remarks
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288520: With LTO and profile-use, enable hotness info in opt
remarks (authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D27332?vs=80026=80088#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27332
Files:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/darwin-ld.c
Index: cfe/trunk/test/Driver/darwin-ld.c
===
--- cfe/trunk/test/Driver/darwin-ld.c
+++ cfe/trunk/test/Driver/darwin-ld.c
@@ -332,7 +332,12 @@
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-### -o foo/bar.out 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT %s < %t.log
// PASS_REMARKS_OUTPUT: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"foo/bar.out.opt.yaml"
+// PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks-with-hotness
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-### 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT_NO_O %s < %t.log
// PASS_REMARKS_OUTPUT_NO_O: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"a.out.opt.yaml"
+
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-fprofile-instr-use=blah -### -o foo/bar.out 2> %t.log
+// RUN: FileCheck -check-prefix=PASS_REMARKS_WITH_HOTNESS %s < %t.log
+// PASS_REMARKS_WITH_HOTNESS: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"foo/bar.out.opt.yaml" "-mllvm" "-lto-pass-remarks-with-hotness"
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -3644,6 +3644,19 @@
return VersionTuple();
}
+static Arg *getLastProfileUseArg(const ArgList ) {
+ auto *ProfileUseArg = Args.getLastArg(
+ options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
+ options::OPT_fprofile_use, options::OPT_fprofile_use_EQ,
+ options::OPT_fno_profile_instr_use);
+
+ if (ProfileUseArg &&
+ ProfileUseArg->getOption().matches(options::OPT_fno_profile_instr_use))
+ProfileUseArg = nullptr;
+
+ return ProfileUseArg;
+}
+
static void addPGOAndCoverageFlags(Compilation , const Driver ,
const InputInfo , const ArgList
,
ArgStringList ) {
@@ -3668,13 +3681,7 @@
D.Diag(diag::err_drv_argument_not_allowed_with)
<< PGOGenerateArg->getSpelling() << ProfileGenerateArg->getSpelling();
- auto *ProfileUseArg = Args.getLastArg(
- options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
- options::OPT_fprofile_use, options::OPT_fprofile_use_EQ,
- options::OPT_fno_profile_instr_use);
- if (ProfileUseArg &&
- ProfileUseArg->getOption().matches(options::OPT_fno_profile_instr_use))
-ProfileUseArg = nullptr;
+ auto *ProfileUseArg = getLastProfileUseArg(Args);
if (PGOGenerateArg && ProfileUseArg)
D.Diag(diag::err_drv_argument_not_allowed_with)
@@ -8436,6 +8443,11 @@
F = Output.getFilename();
F += ".opt.yaml";
CmdArgs.push_back(Args.MakeArgString(F));
+
+if (getLastProfileUseArg(Args)) {
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-lto-pass-remarks-with-hotness");
+}
}
// It seems that the 'e' option is completely ignored for dynamic executables
Index: cfe/trunk/test/Driver/darwin-ld.c
===
--- cfe/trunk/test/Driver/darwin-ld.c
+++ cfe/trunk/test/Driver/darwin-ld.c
@@ -332,7 +332,12 @@
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### -o foo/bar.out 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT %s < %t.log
// PASS_REMARKS_OUTPUT: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml"
+// PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks-with-hotness
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT_NO_O %s < %t.log
// PASS_REMARKS_OUTPUT_NO_O: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "a.out.opt.yaml"
+
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -fprofile-instr-use=blah -### -o foo/bar.out 2> %t.log
+// RUN: FileCheck -check-prefix=PASS_REMARKS_WITH_HOTNESS %s < %t.log
+// PASS_REMARKS_WITH_HOTNESS: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml" "-mllvm" "-lto-pass-remarks-with-hotness"
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -3644,6 +3644,19 @@
return VersionTuple();
}
+static Arg *getLastProfileUseArg(const ArgList ) {
+ auto *ProfileUseArg = Args.getLastArg(
+ options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
+ options::OPT_fprofile_use,
[PATCH] D27332: With LTO and profile-use, enable hotness info in opt remarks
anemet added a comment. In https://reviews.llvm.org/D27332#611769, @mehdi_amini wrote: > (You have a typo in the decription, you may want to fix it before commit) Got it. Thanks for the reviews! https://reviews.llvm.org/D27332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27332: With LTO and profile-use, enable hotness info in opt remarks
anemet created this revision.
anemet added reviewers: hfinkel, mehdi_amini.
anemet added a subscriber: cfe-commits.
This is to match the behavior of non-LTO; when -fsave-optmization-record
is passed and PGO is available we enable the generation of hotness
information in the optimization records.
https://reviews.llvm.org/D27332
Files:
lib/Driver/Tools.cpp
test/Driver/darwin-ld.c
Index: test/Driver/darwin-ld.c
===
--- test/Driver/darwin-ld.c
+++ test/Driver/darwin-ld.c
@@ -332,7 +332,12 @@
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-### -o foo/bar.out 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT %s < %t.log
// PASS_REMARKS_OUTPUT: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"foo/bar.out.opt.yaml"
+// PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks-with-hotness
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-### 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT_NO_O %s < %t.log
// PASS_REMARKS_OUTPUT_NO_O: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"a.out.opt.yaml"
+
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record
-fprofile-instr-use=blah -### -o foo/bar.out 2> %t.log
+// RUN: FileCheck -check-prefix=PASS_REMARKS_WITH_HOTNESS %s < %t.log
+// PASS_REMARKS_WITH_HOTNESS: "-mllvm" "-lto-pass-remarks-output" "-mllvm"
"foo/bar.out.opt.yaml" "-mllvm" "-lto-pass-remarks-with-hotness"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3641,6 +3641,19 @@
return VersionTuple();
}
+static Arg *getLastProfileUseArg(const ArgList ) {
+ auto *ProfileUseArg = Args.getLastArg(
+ options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
+ options::OPT_fprofile_use, options::OPT_fprofile_use_EQ,
+ options::OPT_fno_profile_instr_use);
+
+ if (ProfileUseArg &&
+ ProfileUseArg->getOption().matches(options::OPT_fno_profile_instr_use))
+ProfileUseArg = nullptr;
+
+ return ProfileUseArg;
+}
+
static void addPGOAndCoverageFlags(Compilation , const Driver ,
const InputInfo , const ArgList
,
ArgStringList ) {
@@ -3665,13 +3678,7 @@
D.Diag(diag::err_drv_argument_not_allowed_with)
<< PGOGenerateArg->getSpelling() << ProfileGenerateArg->getSpelling();
- auto *ProfileUseArg = Args.getLastArg(
- options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
- options::OPT_fprofile_use, options::OPT_fprofile_use_EQ,
- options::OPT_fno_profile_instr_use);
- if (ProfileUseArg &&
- ProfileUseArg->getOption().matches(options::OPT_fno_profile_instr_use))
-ProfileUseArg = nullptr;
+ auto *ProfileUseArg = getLastProfileUseArg(Args);
if (PGOGenerateArg && ProfileUseArg)
D.Diag(diag::err_drv_argument_not_allowed_with)
@@ -8433,6 +8440,11 @@
F = Output.getFilename();
F += ".opt.yaml";
CmdArgs.push_back(Args.MakeArgString(F));
+
+if (getLastProfileUseArg(Args)) {
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-lto-pass-remarks-with-hotness");
+}
}
// It seems that the 'e' option is completely ignored for dynamic executables
Index: test/Driver/darwin-ld.c
===
--- test/Driver/darwin-ld.c
+++ test/Driver/darwin-ld.c
@@ -332,7 +332,12 @@
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### -o foo/bar.out 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT %s < %t.log
// PASS_REMARKS_OUTPUT: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml"
+// PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks-with-hotness
// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### 2> %t.log
// RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT_NO_O %s < %t.log
// PASS_REMARKS_OUTPUT_NO_O: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "a.out.opt.yaml"
+
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -fprofile-instr-use=blah -### -o foo/bar.out 2> %t.log
+// RUN: FileCheck -check-prefix=PASS_REMARKS_WITH_HOTNESS %s < %t.log
+// PASS_REMARKS_WITH_HOTNESS: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml" "-mllvm" "-lto-pass-remarks-with-hotness"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3641,6 +3641,19 @@
return VersionTuple();
}
+static Arg *getLastProfileUseArg(const ArgList ) {
+ auto *ProfileUseArg = Args.getLastArg(
+ options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
+ options::OPT_fprofile_use, options::OPT_fprofile_use_EQ,
+ options::OPT_fno_profile_instr_use);
+
+ if (ProfileUseArg &&
+
