Skip to content

[VPlan] Use pragma pack(1) for VPIRFlags on AIX.#184687

Merged
fhahn merged 3 commits intollvm:mainfrom
fhahn:vplan-aix-packing
Mar 5, 2026
Merged

[VPlan] Use pragma pack(1) for VPIRFlags on AIX.#184687
fhahn merged 3 commits intollvm:mainfrom
fhahn:vplan-aix-packing

Conversation

@fhahn
Copy link
Contributor

@fhahn fhahn commented Mar 4, 2026

Some compilers (e.g. on AIX) do not pack by default. Use LLVM_PACKED to ensure the VPIRFlags struct is packed as expected on all platforms.

This matches what we already do in other places for AIX, e.g. in llvm/include/llvm/CodeGen/SelectionDAGNodes.h (added in 844a02e), although it uses the more general LLVM_PACKED unconditionally

Except for GCC; by default, AIX compilers store bit-fields in 4-byte
words. This means that VPIRFlags is larger than expected.

Use pack(1) to ensure smaller packing, matching other platforms.

This matches what we already do in other places for AIX, e.g. in
llvm/include/llvm/CodeGen/SelectionDAGNodes.h (added in
844a02e)
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2026

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

Changes

Except for GCC; by default, AIX compilers store bit-fields in 4-byte words. This means that VPIRFlags is larger than expected.

Use pack(1) to ensure smaller packing, matching other platforms.

This matches what we already do in other places for AIX, e.g. in llvm/include/llvm/CodeGen/SelectionDAGNodes.h (added in 844a02e)


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+13)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 41eef2a368343..d2ea1b362ccba 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -668,6 +668,16 @@ class VPSingleDefRecipe : public VPRecipeBase, public VPRecipeValue {
 };
 
 /// Class to record and manage LLVM IR flags.
+#if defined(_AIX) && (!defined(__GNUC__) || defined(__clang__))
+// Except for GCC; by default, AIX compilers store bit-fields in 4-byte words
+// and give the `pack` pragma push semantics.
+#define BEGIN_ONE_BYTE_PACK() _Pragma("pack(1)")
+#define END_ONE_BYTE_PACK() _Pragma("pack(pop)")
+#else
+#define BEGIN_ONE_BYTE_PACK()
+#define END_ONE_BYTE_PACK()
+#endif
+BEGIN_ONE_BYTE_PACK()
 class VPIRFlags {
   enum class OperationType : unsigned char {
     Cmp,
@@ -1070,6 +1080,9 @@ class VPIRFlags {
   void printFlags(raw_ostream &O) const;
 #endif
 };
+END_ONE_BYTE_PACK()
+#undef BEGIN_ONE_BYTE_PACK
+#undef END_ONE_BYTE_PACK
 
 static_assert(sizeof(VPIRFlags) <= 3, "VPIRFlags should not grow");
 

@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2026

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Except for GCC; by default, AIX compilers store bit-fields in 4-byte words. This means that VPIRFlags is larger than expected.

Use pack(1) to ensure smaller packing, matching other platforms.

This matches what we already do in other places for AIX, e.g. in llvm/include/llvm/CodeGen/SelectionDAGNodes.h (added in 844a02e)


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+13)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 41eef2a368343..d2ea1b362ccba 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -668,6 +668,16 @@ class VPSingleDefRecipe : public VPRecipeBase, public VPRecipeValue {
 };
 
 /// Class to record and manage LLVM IR flags.
+#if defined(_AIX) && (!defined(__GNUC__) || defined(__clang__))
+// Except for GCC; by default, AIX compilers store bit-fields in 4-byte words
+// and give the `pack` pragma push semantics.
+#define BEGIN_ONE_BYTE_PACK() _Pragma("pack(1)")
+#define END_ONE_BYTE_PACK() _Pragma("pack(pop)")
+#else
+#define BEGIN_ONE_BYTE_PACK()
+#define END_ONE_BYTE_PACK()
+#endif
+BEGIN_ONE_BYTE_PACK()
 class VPIRFlags {
   enum class OperationType : unsigned char {
     Cmp,
@@ -1070,6 +1080,9 @@ class VPIRFlags {
   void printFlags(raw_ostream &O) const;
 #endif
 };
+END_ONE_BYTE_PACK()
+#undef BEGIN_ONE_BYTE_PACK
+#undef END_ONE_BYTE_PACK
 
 static_assert(sizeof(VPIRFlags) <= 3, "VPIRFlags should not grow");
 

@fhahn
Copy link
Contributor Author

fhahn commented Mar 4, 2026

@kkwli @hubert-reinterpretcast I think this should fix the build failure mentioned in #181571 for AIX, but I am not able to verify locally. Would be great if you could help

@midhuncodes7
Copy link
Contributor

midhuncodes7 commented Mar 5, 2026

I added this fix to my branch to test on AIX but ended up with different errors:

/home/midhun7/llvm/forked/llvm-project/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp:151:10: error: no member named 'setStartIndex' in 'llvm::VPScalarIVStepsRecipe'
  151 |   Steps->setStartIndex(StartIndex);
      |   ~~~~~  ^
/home/midhun7/llvm/forked/llvm-project/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp:615:37: error: no member named 'getInductionOpcode' in 'llvm::VPScalarIVStepsRecipe'
  615 |           if (!StartIndex && Steps->getInductionOpcode() == Instruction::FSub)
      |                              ~~~~~  ^
/home/midhun7/llvm/forked/llvm-project/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp:619:26: error: no member named 'getInductionOpcode' in 'llvm::VPScalarIVStepsRecipe'
  619 |           AddOp = Steps->getInductionOpcode();
      |                   ~~~~~  ^
/home/midhun7/llvm/forked/llvm-project/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp:635:16: error: no member named 'setStartIndex' in 'llvm::VPScalarIVStepsRecipe'
  635 |         Steps->setStartIndex(LaneOffset);
      |         ~~~~~  ^
4 errors generated.

On commit:

commit d316fb0797045d6f85b237aac18fabbed42e5d09 (HEAD -> main, origin/main, origin/HEAD)
Author: Florian Hahn <flo@fhahn.com>
Date:   Thu Mar 5 12:42:20 2026 +0000

    [VPlan] Replicate VPScalarIVStepsRecipe by VF outside replicate regions. (#170053)

@fhahn
Copy link
Contributor Author

fhahn commented Mar 5, 2026

@midhuncodes7 is this with a clean build? So far looks like this commit is fine on the build bots.

@midhuncodes7
Copy link
Contributor

midhuncodes7 commented Mar 5, 2026

The changes introduced in this PR: #170053 along with the fix provided in the current PR was built clean and results with the posted error.

Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

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

LG. I can successfully build the patch on AIX without encountering the previously described error.

Copy link
Contributor Author

@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.

LG. I can successfully build the patch on AIX without encountering the previously described error.

@kkwli thanks for checking, I updated the code to just use LLVM_PACKED_START/END, so this is now done unconditionally

@fhahn fhahn enabled auto-merge (squash) March 5, 2026 19:00
@fhahn fhahn disabled auto-merge March 5, 2026 19:00
@fhahn fhahn enabled auto-merge (squash) March 5, 2026 19:01
Copy link
Contributor

@artagnon artagnon left a comment

Choose a reason for hiding this comment

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

If this fixes the issue, well and good. LGTM, thanks!

@fhahn fhahn merged commit 5e88b80 into llvm:main Mar 5, 2026
9 of 10 checks passed
@fhahn fhahn deleted the vplan-aix-packing branch March 5, 2026 19:30
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Mar 5, 2026
Some compilers (e.g. on AIX) do not pack by default. Use LLVM_PACKED to
ensure the VPIRFlags struct is packed as expected on all platforms.

This matches what we already do in other places for AIX, e.g. in
llvm/include/llvm/CodeGen/SelectionDAGNodes.h (added in
844a02e), although it uses the more
general LLVM_PACKED unconditionally

PR: llvm/llvm-project#184687
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.

5 participants