Skip to content

[LV][EVL] Refine the constructors of EVL recipe to use call by reference. NFC #100088

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

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

Mel-Chen
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Mel Chen (Mel-Chen)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/100088.diff

3 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+13-14)
  • (modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+3-3)
  • (modified) llvm/unittests/Transforms/Vectorize/VPlanTest.cpp (+2-2)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 0b596e7e4f633..6b78fa31e59dc 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -2246,12 +2246,12 @@ class VPReductionRecipe : public VPSingleDefRecipe {
 /// The Operands are {ChainOp, VecOp, EVL, [Condition]}.
 class VPReductionEVLRecipe : public VPReductionRecipe {
 public:
-  VPReductionEVLRecipe(VPReductionRecipe *R, VPValue *EVL, VPValue *CondOp)
+  VPReductionEVLRecipe(VPReductionRecipe &R, VPValue &EVL, VPValue *CondOp)
       : VPReductionRecipe(
-            VPDef::VPReductionEVLSC, R->getRecurrenceDescriptor(),
-            cast_or_null<Instruction>(R->getUnderlyingValue()),
-            ArrayRef<VPValue *>({R->getChainOp(), R->getVecOp(), EVL}), CondOp,
-            R->isOrdered()) {}
+            VPDef::VPReductionEVLSC, R.getRecurrenceDescriptor(),
+            cast_or_null<Instruction>(R.getUnderlyingValue()),
+            ArrayRef<VPValue *>({R.getChainOp(), R.getVecOp(), &EVL}), CondOp,
+            R.isOrdered()) {}
 
   ~VPReductionEVLRecipe() override = default;
 
@@ -2558,10 +2558,10 @@ struct VPWidenLoadRecipe final : public VPWidenMemoryRecipe, public VPValue {
 /// using the address to load from, the explicit vector length and an optional
 /// mask.
 struct VPWidenLoadEVLRecipe final : public VPWidenMemoryRecipe, public VPValue {
-  VPWidenLoadEVLRecipe(VPWidenLoadRecipe *L, VPValue *EVL, VPValue *Mask)
-      : VPWidenMemoryRecipe(VPDef::VPWidenLoadEVLSC, L->getIngredient(),
-                            {L->getAddr(), EVL}, L->isConsecutive(),
-                            L->isReverse(), L->getDebugLoc()),
+  VPWidenLoadEVLRecipe(VPWidenLoadRecipe &L, VPValue &EVL, VPValue *Mask)
+      : VPWidenMemoryRecipe(VPDef::VPWidenLoadEVLSC, L.getIngredient(),
+                            {L.getAddr(), &EVL}, L.isConsecutive(),
+                            L.isReverse(), L.getDebugLoc()),
         VPValue(this, &getIngredient()) {
     setMask(Mask);
   }
@@ -2634,11 +2634,10 @@ struct VPWidenStoreRecipe final : public VPWidenMemoryRecipe {
 /// using the value to store, the address to store to, the explicit vector
 /// length and an optional mask.
 struct VPWidenStoreEVLRecipe final : public VPWidenMemoryRecipe {
-  VPWidenStoreEVLRecipe(VPWidenStoreRecipe *S, VPValue *EVL, VPValue *Mask)
-      : VPWidenMemoryRecipe(VPDef::VPWidenStoreEVLSC, S->getIngredient(),
-                            {S->getAddr(), S->getStoredValue(), EVL},
-                            S->isConsecutive(), S->isReverse(),
-                            S->getDebugLoc()) {
+  VPWidenStoreEVLRecipe(VPWidenStoreRecipe &S, VPValue &EVL, VPValue *Mask)
+      : VPWidenMemoryRecipe(VPDef::VPWidenStoreEVLSC, S.getIngredient(),
+                            {S.getAddr(), S.getStoredValue(), &EVL},
+                            S.isConsecutive(), S.isReverse(), S.getDebugLoc()) {
     setMask(Mask);
   }
 
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index c91fd0f118e31..045f6c356669f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1488,13 +1488,13 @@ bool VPlanTransforms::tryAddExplicitVectorLength(VPlan &Plan) {
       if (auto *MemR = dyn_cast<VPWidenMemoryRecipe>(CurRecipe)) {
         VPValue *NewMask = GetNewMask(MemR->getMask());
         if (auto *L = dyn_cast<VPWidenLoadRecipe>(MemR))
-          NewRecipe = new VPWidenLoadEVLRecipe(L, VPEVL, NewMask);
+          NewRecipe = new VPWidenLoadEVLRecipe(*L, *VPEVL, NewMask);
         else if (auto *S = dyn_cast<VPWidenStoreRecipe>(MemR))
-          NewRecipe = new VPWidenStoreEVLRecipe(S, VPEVL, NewMask);
+          NewRecipe = new VPWidenStoreEVLRecipe(*S, *VPEVL, NewMask);
         else
           llvm_unreachable("unsupported recipe");
       } else if (auto *RedR = dyn_cast<VPReductionRecipe>(CurRecipe)) {
-        NewRecipe = new VPReductionEVLRecipe(RedR, VPEVL,
+        NewRecipe = new VPReductionEVLRecipe(*RedR, *VPEVL,
                                              GetNewMask(RedR->getCondOp()));
       }
 
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index d9d6789134d88..9cf9060458bc9 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -1140,7 +1140,7 @@ TEST(VPRecipeTest, MayHaveSideEffectsAndMayReadWriteMemory) {
     VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
                              &VecOp, false);
     VPValue EVL;
-    VPReductionEVLRecipe EVLRecipe(&Recipe, &EVL, &CondOp);
+    VPReductionEVLRecipe EVLRecipe(Recipe, EVL, &CondOp);
     EXPECT_FALSE(EVLRecipe.mayHaveSideEffects());
     EXPECT_FALSE(EVLRecipe.mayReadFromMemory());
     EXPECT_FALSE(EVLRecipe.mayWriteToMemory());
@@ -1495,7 +1495,7 @@ TEST(VPRecipeTest, CastVPReductionEVLRecipeToVPUser) {
   VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
                            &VecOp, false);
   VPValue EVL;
-  VPReductionEVLRecipe EVLRecipe(&Recipe, &EVL, &CondOp);
+  VPReductionEVLRecipe EVLRecipe(Recipe, EVL, &CondOp);
   EXPECT_TRUE(isa<VPUser>(&EVLRecipe));
   VPRecipeBase *BaseR = &EVLRecipe;
   EXPECT_TRUE(isa<VPUser>(BaseR));

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Mel-Chen Mel-Chen merged commit 3834523 into llvm:main Jul 26, 2024
10 checks passed
@Mel-Chen Mel-Chen deleted the evl-constructor-nfc branch July 26, 2024 08:50
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 26, 2024

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-5 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/2702

Here is the relevant piece of the build log for the reference:

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang Tools :: clang-doc/basic-project.test' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Emiting docs in html format.
Mapping decls...
Collecting infos...
Reducing 5 infos...
Generating docs...
Generating assets for docs...

--
Command Output (stderr):
--
RUN: at line 1: rm -rf /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp && mkdir -p /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
+ rm -rf /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp
+ mkdir -p /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
RUN: at line 2: sed 's|$test_dir|/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc|g' /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json > /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ sed 's|$test_dir|/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc|g' /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json
RUN: at line 3: clang-doc --format=html --output=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ clang-doc --format=html --output=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
[1/3] Processing file /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Rectangle.cpp
[3/3] Processing file /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Calculator.cpp
[2/3] Processing file /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Circle.cpp
RUN: at line 4: FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/index_json.js -check-prefix=JSON-INDEX
+ FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/index_json.js -check-prefix=JSON-INDEX
RUN: at line 5: FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html -check-prefix=HTML-SHAPE
+ FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html -check-prefix=HTML-SHAPE
�[1m/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test:64:16: �[0m�[0;1;31merror: �[0m�[1mHTML-SHAPE: expected string not found in input
�[0m// HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">area</h3>
�[0;1;32m               ^
�[0m�[1m/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html:27:53: �[0m�[0;1;30mnote: �[0m�[1mscanning from here
�[0m <p>Defined at line 13 of file ./include/Shape.h</p>
�[0;1;32m                                                    ^
�[0m�[1m/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html:47:41: �[0m�[0;1;30mnote: �[0m�[1mpossible intended match here
�[0m <a href="#12896F9255F880ECD4A6482CCFA58B238FA2CC49">area</a>
�[0;1;32m                                        ^
�[0m
Input file: /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html
Check file: /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test

-dump-input=help explains the following input dump.

Input was:
<<<<<<
�[1m�[0m�[0;1;30m            1: �[0m�[1m�[0;1;46m<!DOCTYPE html> �[0m
�[0;1;30m            2: �[0m�[1m�[0;1;46m<meta charset="utf-8"/> �[0m
�[0;1;30m            3: �[0m�[1m�[0;1;46m<title>class Shape</title> �[0m
�[0;1;30m            4: �[0m�[1m�[0;1;46m<link rel="stylesheet" href="../clang-doc-default-stylesheet.css"/> �[0m
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants