[clang] 475ceca - [clang] Increase VecLib bitfield size to 4 bits in CodeGenOptions.def (#108804)

[via cfe-commits]

Author: Mainak Sil
Date: 2024-09-18T11:29:49-04:00
New Revision: 475ceca859233b387c22f13ecef581158ef36346

URL: https://github.com/llvm/llvm-project/commit/475ceca859233b387c22f13ecef581158ef36346
DIFF: https://github.com/llvm/llvm-project/commit/475ceca859233b387c22f13ecef581158ef36346.diff

LOG: [clang] Increase VecLib bitfield size to 4 bits in CodeGenOptions.def (#108804)

Summary:
This PR fixes the issue where the VecLib bitfield in CodeGenOptions.def
is too small to accommodate the increasing number of vector libraries.
Specifically, the bitfield size was previously set to 3, but with the
introduction of more vector libraries (currently 9), the bitfield needed
to be expanded to avoid potential issues in vectorization.

In this PR, I have increased the size of the VecLib bitfield from 3 to 4
to account for the additional libraries. This ensures that all 9 vector
libraries are correctly encoded and available for use without errors.

Changes Made:
Modified: Increased the VecLib bitfield size from 3 to 4 in
clang/include/clang/Basic/CodeGenOptions.def.

Motivation:
This change is necessary to ensure that all vector libraries are
properly represented and selectable. The current limitation of the
VecLib bitfield size was causing some vectorization opportunities to be
lost when more than 3 bits were needed to represent the library options.

Closes:
Fixes https://github.com/llvm/llvm-project/issues/108704

Added: 
    clang/unittests/CodeGen/AllLibrariesFit.cpp
    clang/unittests/CodeGen/EncodingDecodingTest.cpp
    clang/unittests/CodeGen/SimulatedOverflowTest.cpp

Modified: 
    clang/include/clang/Basic/CodeGenOptions.def

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index b600198998d85b..b78ae61e6509ea 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -375,8 +375,18 @@ ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NormalInlining)
 /// The maximum stack size a function can have to be considered for inlining.
 VALUE_CODEGENOPT(InlineMaxStackSize, 32, UINT_MAX)
 
-// Vector functions library to use.
-ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, 3, llvm::driver::VectorLibrary::NoLibrary)
+// Define the number of bits required for the VecLib enum
+#define VECLIB_BIT_COUNT (llvm::countPopulation(llvm::driver::VectorLibrary::MaxLibrary))
+
+// Ensure the VecLib bitfield has enough space for future vector libraries.
+// The number of bits is determined automatically based on the number of enum values.
+static_assert(static_cast<size_t>(llvm::driver::VectorLibrary::MaxLibrary) <= (1 << VECLIB_BIT_COUNT),
+              "VecLib bitfield size is too small to accommodate all vector libraries.");
+              
+// VecLib definition in CodeGenOptions.def
+ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, VECLIB_BIT_COUNT, llvm::driver::VectorLibrary::NoLibrary)
+
+#undef VECLIB_BIT_COUNT
 
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)

diff  --git a/clang/unittests/CodeGen/AllLibrariesFit.cpp b/clang/unittests/CodeGen/AllLibrariesFit.cpp
new file mode 100644
index 00000000000000..dfe63b557729ee
--- /dev/null
+++ b/clang/unittests/CodeGen/AllLibrariesFit.cpp
@@ -0,0 +1,10 @@
+#include "clang/Basic/CodeGenOptions.h"
+#include "llvm/Driver/Options.h"
+#include "gtest/gtest.h"
+
+TEST(VecLibBitfieldTest, AllLibrariesFit) {
+  // We expect that all vector libraries fit in the bitfield size
+  EXPECT_LE(static_cast<size_t>(llvm::driver::VectorLibrary::MaxLibrary),
+            (1 << VECLIB_BIT_COUNT))
+      << "VecLib bitfield size is too small!";
+ }

diff  --git a/clang/unittests/CodeGen/EncodingDecodingTest.cpp b/clang/unittests/CodeGen/EncodingDecodingTest.cpp
new file mode 100644
index 00000000000000..67c89ef07c428b
--- /dev/null
+++ b/clang/unittests/CodeGen/EncodingDecodingTest.cpp
@@ -0,0 +1,17 @@
+TEST(VecLibBitfieldTest, EncodingDecodingTest) {
+  clang::CodeGenOptions Opts;
+
+  // Test encoding and decoding for each vector library
+  for (int i = static_cast<int>(llvm::driver::VectorLibrary::Accelerate);
+       i <= static_cast<int>(llvm::driver::VectorLibrary::MaxLibrary); ++i) {
+
+    Opts.VecLib = static_cast<llvm::driver::VectorLibrary>(i);
+
+    // Encode and then decode
+    llvm::driver::VectorLibrary decodedValue =
+        static_cast<llvm::driver::VectorLibrary>(Opts.VecLib);
+
+    EXPECT_EQ(decodedValue, Opts.VecLib)
+        << "Encoding/Decoding failed for vector library " << i;
+  }
+}

diff  --git a/clang/unittests/CodeGen/SimulatedOverflowTest.cpp b/clang/unittests/CodeGen/SimulatedOverflowTest.cpp
new file mode 100644
index 00000000000000..acfeaf7498b6d0
--- /dev/null
+++ b/clang/unittests/CodeGen/SimulatedOverflowTest.cpp
@@ -0,0 +1,26 @@
+// Simulate the addition of a new library without increasing the bitfield size
+enum class SimulatedVectorLibrary {
+  Accelerate = 0,
+  LIBMVEC,
+  MASSV,
+  SVML,
+  SLEEF,
+  Darwin_libsystem_m,
+  ArmPL,
+  AMDLIBM,
+  NoLibrary,
+  // Simulate new addition
+  NewLibrary,
+  MaxLibrary
+};
+
+#define SIMULATED_VECLIB_BIT_COUNT                                             \
+  4 // The current bitfield size (should be 4 for 9 options)
+
+TEST(VecLibBitfieldTest, SimulatedOverflowTest) {
+  // Simulate the addition of a new library and check if the bitfield size is
+  // sufficient
+  EXPECT_LE(static_cast<size_t>(SimulatedVectorLibrary::MaxLibrary),
+            (1 << SIMULATED_VECLIB_BIT_COUNT))
+      << "Simulated VecLib bitfield size overflow!";
+}