-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Introduce -fexperimental-loop-fuse to clang and flang #142686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e2f5344
7b4fdc4
fe53e39
ab202f0
3f88802
9f264b4
76780b0
00d5ca5
551813b
a007cea
4c72de9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
! RUN: %flang -### -S -fexperimental-loop-fusion %s 2>&1 | FileCheck -check-prefix=CHECK-LOOP-FUSE %s | ||
! RUN: %flang -### -S -fno-experimental-loop-fusion %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -O0 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -O1 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! RUN: %flang -### -S -Oz %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE %s | ||
! CHECK-LOOP-FUSE: "-fexperimental-loop-fusion" | ||
! CHECK-NO-LOOP-FUSE-NOT: "-fexperimental-loop-fusion" | ||
! RUN: %flang_fc1 -emit-llvm -O2 -fexperimental-loop-fusion -mllvm -print-pipeline-passes -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-LOOP-FUSE-PASS %s | ||
! RUN: %flang_fc1 -emit-llvm -O2 -fno-experimental-loop-fusion -mllvm -print-pipeline-passes -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-NO-LOOP-FUSE-PASS %s | ||
! CHECK-LOOP-FUSE-PASS: loop-fusion | ||
! CHECK-NO-LOOP-FUSE-PASS-NOT: loop-fusion | ||
|
||
program test | ||
end program |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,9 @@ class PipelineTuningOptions { | |
/// false. | ||
bool LoopInterchange; | ||
|
||
/// Tuning option to enable/disable loop fusion. Its default value is false. | ||
bool LoopFusion; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell, this default is never actually set anywhere. There is no assignment in the PipelineTuningOptions ctor. (Probably that ctor should be replaced with direct struct member initialization...) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In This is required at each site which is using I am inclined to follow the way LoopInterchange is initialized (and thus move the definition of LoopFusion from What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Why can't you do the initialization to false in PassBuilderPipelines? Isn't this just the default value that later gets overwritten by the code in NewPMDriver? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is another option. #158844. Can you please have a quick look? |
||
|
||
/// Tuning option to forget all SCEV loops in LoopUnroll. Its default value | ||
/// is that of the flag: `-forget-scev-loop-unroll`. | ||
bool ForgetAllSCEVInLoopUnroll; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should check that
-fno-experimental-loop-fusion
is also recognized by clang.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is usually done or necessary. I checked other options, which are introduced by
addOptInFlag
, namelyfunique_internal_linkage_names
,fseparate_named_sections
,fatomic_ignore_denormal_mode
,fsplit_stack
, and many others, and I don't see any precedence of having a test which checks if the negative options are recognized.Do we want to make an exception for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine not to test
-fno-experimental-loop-fusion
. I also don't mind if the author adds it and adds a CHECK-NOT.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was partly basing this on the fact that the loop-interchange above does check for both, but if I have not checked if that uses
addOptInFlag
. If there is precedence for not testing it, that's fine too.