Skip to content

Conversation

@zahiraam
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Feb 21, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff d8eb4ac41d881a19bea7673d753ba92e6a11f5d6 d13d540ccc0b09e3779eb1f55d86afa710f43666 --extensions cpp,h -- clang/include/clang/AST/ASTContext.h clang/lib/CodeGen/CodeGenModule.cpp clang/lib/Parse/ParsePragma.cpp clang/test/CodeGenCXX/sections.cpp llvm/include/llvm/IR/GlobalVariable.h llvm/include/llvm/MC/SectionKind.h llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp llvm/lib/IR/Globals.cpp llvm/lib/Target/TargetLoweringObjectFile.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp
index fe3267f530..a2bf44f9b5 100644
--- a/clang/lib/Parse/ParsePragma.cpp
+++ b/clang/lib/Parse/ParsePragma.cpp
@@ -1103,17 +1103,17 @@ bool Parser::HandlePragmaMSSection(StringRef PragmaName,
       return false;
     }
     ASTContext::PragmaSectionFlag Flag =
-      llvm::StringSwitch<ASTContext::PragmaSectionFlag>(
-      Tok.getIdentifierInfo()->getName())
-      .Case("read", ASTContext::PSF_Read)
-      .Case("write", ASTContext::PSF_Write)
-      .Case("execute", ASTContext::PSF_Execute)
-      .Case("shared", ASTContext::PSF_Shared)
-      .Case("nopage", ASTContext::PSF_Invalid)
-      .Case("nocache", ASTContext::PSF_Invalid)
-      .Case("discard", ASTContext::PSF_Invalid)
-      .Case("remove", ASTContext::PSF_Invalid)
-      .Default(ASTContext::PSF_None);
+        llvm::StringSwitch<ASTContext::PragmaSectionFlag>(
+            Tok.getIdentifierInfo()->getName())
+            .Case("read", ASTContext::PSF_Read)
+            .Case("write", ASTContext::PSF_Write)
+            .Case("execute", ASTContext::PSF_Execute)
+            .Case("shared", ASTContext::PSF_Shared)
+            .Case("nopage", ASTContext::PSF_Invalid)
+            .Case("nocache", ASTContext::PSF_Invalid)
+            .Case("discard", ASTContext::PSF_Invalid)
+            .Case("remove", ASTContext::PSF_Invalid)
+            .Default(ASTContext::PSF_None);
     if (Flag == ASTContext::PSF_None || Flag == ASTContext::PSF_Invalid) {
       PP.Diag(PragmaLocation, Flag == ASTContext::PSF_None
                                   ? diag::warn_pragma_invalid_specific_action
diff --git a/llvm/include/llvm/IR/GlobalVariable.h b/llvm/include/llvm/IR/GlobalVariable.h
index af7e49c2ce..0bf3f4d42b 100644
--- a/llvm/include/llvm/IR/GlobalVariable.h
+++ b/llvm/include/llvm/IR/GlobalVariable.h
@@ -62,8 +62,8 @@ public:
   GlobalVariable(Type *Ty, bool isConstant, LinkageTypes Linkage,
                  Constant *Initializer = nullptr, const Twine &Name = "",
                  ThreadLocalMode = NotThreadLocal, unsigned AddressSpace = 0,
-                 bool isExternallyInitialized = false, bool isExecuteGlobal = false,
-                 bool isSharedGlobal = false);
+                 bool isExternallyInitialized = false,
+                 bool isExecuteGlobal = false, bool isSharedGlobal = false);
   /// GlobalVariable ctor - This creates a global and inserts it before the
   /// specified other global.
   GlobalVariable(Module &M, Type *Ty, bool isConstant, LinkageTypes Linkage,
@@ -186,7 +186,7 @@ public:
   bool isExecute() const { return isExecuteGlobal; }
   bool isShared() const { return isSharedGlobal; }
   void setExecute(bool Val) { isExecuteGlobal = Val; }
-  void setShared(bool Val) { isSharedGlobal = Val;  }
+  void setShared(bool Val) { isSharedGlobal = Val; }
 
   /// copyAttributesFrom - copy all additional attributes (those not needed to
   /// create a GlobalVariable) from the GlobalVariable Src to this one.
diff --git a/llvm/include/llvm/MC/SectionKind.h b/llvm/include/llvm/MC/SectionKind.h
index 0bc4f5ad9a..1f1cae647f 100644
--- a/llvm/include/llvm/MC/SectionKind.h
+++ b/llvm/include/llvm/MC/SectionKind.h
@@ -23,7 +23,7 @@ class SectionKind {
   enum Kind {
     /// Metadata - Debug info sections or other metadata.
     Metadata,
-    /// Shared 
+    /// Shared
     Shared,
     /// Exclude - This section will be excluded from the final executable or
     /// shared library. Only valid for ELF / COFF targets.
@@ -32,93 +32,94 @@ class SectionKind {
     /// Text - Text section, used for functions and other executable code.
     Text,
 
-           /// ExecuteOnly, Text section that is not readable.
-           ExecuteOnly,
+    /// ExecuteOnly, Text section that is not readable.
+    ExecuteOnly,
 
     /// ReadOnly - Data that is never written to at program runtime by the
     /// program or the dynamic linker.  Things in the top-level readonly
     /// SectionKind are not mergeable.
     ReadOnly,
 
-        /// MergableCString - Any null-terminated string which allows merging.
-        /// These values are known to end in a nul value of the specified size,
-        /// not otherwise contain a nul value, and be mergable.  This allows the
-        /// linker to unique the strings if it so desires.
+    /// MergableCString - Any null-terminated string which allows merging.
+    /// These values are known to end in a nul value of the specified size,
+    /// not otherwise contain a nul value, and be mergable.  This allows the
+    /// linker to unique the strings if it so desires.
 
-           /// Mergeable1ByteCString - 1 byte mergable, null terminated, string.
-           Mergeable1ByteCString,
+    /// Mergeable1ByteCString - 1 byte mergable, null terminated, string.
+    Mergeable1ByteCString,
 
-           /// Mergeable2ByteCString - 2 byte mergable, null terminated, string.
-           Mergeable2ByteCString,
+    /// Mergeable2ByteCString - 2 byte mergable, null terminated, string.
+    Mergeable2ByteCString,
 
-           /// Mergeable4ByteCString - 4 byte mergable, null terminated, string.
-           Mergeable4ByteCString,
+    /// Mergeable4ByteCString - 4 byte mergable, null terminated, string.
+    Mergeable4ByteCString,
 
-        /// MergeableConst - These are sections for merging fixed-length
-        /// constants together.  For example, this can be used to unique
-        /// constant pool entries etc.
+    /// MergeableConst - These are sections for merging fixed-length
+    /// constants together.  For example, this can be used to unique
+    /// constant pool entries etc.
 
-            /// MergeableConst4 - This is a section used by 4-byte constants,
-            /// for example, floats.
-            MergeableConst4,
+    /// MergeableConst4 - This is a section used by 4-byte constants,
+    /// for example, floats.
+    MergeableConst4,
 
-            /// MergeableConst8 - This is a section used by 8-byte constants,
-            /// for example, doubles.
-            MergeableConst8,
+    /// MergeableConst8 - This is a section used by 8-byte constants,
+    /// for example, doubles.
+    MergeableConst8,
 
-            /// MergeableConst16 - This is a section used by 16-byte constants,
-            /// for example, vectors.
-            MergeableConst16,
+    /// MergeableConst16 - This is a section used by 16-byte constants,
+    /// for example, vectors.
+    MergeableConst16,
 
-            /// MergeableConst32 - This is a section used by 32-byte constants,
-            /// for example, vectors.
-            MergeableConst32,
+    /// MergeableConst32 - This is a section used by 32-byte constants,
+    /// for example, vectors.
+    MergeableConst32,
 
     /// Writeable - This is the base of all segments that need to be written
     /// to during program runtime.
 
-       /// ThreadLocal - This is the base of all TLS segments.  All TLS
-       /// objects must be writeable, otherwise there is no reason for them to
-       /// be thread local!
+    /// ThreadLocal - This is the base of all TLS segments.  All TLS
+    /// objects must be writeable, otherwise there is no reason for them to
+    /// be thread local!
 
-           /// ThreadBSS - Zero-initialized TLS data objects.
-           ThreadBSS,
+    /// ThreadBSS - Zero-initialized TLS data objects.
+    ThreadBSS,
 
-           /// ThreadData - Initialized TLS data objects.
-           ThreadData,
+    /// ThreadData - Initialized TLS data objects.
+    ThreadData,
 
-           /// ThreadBSSLocal - Zero-initialized TLS data objects with local linkage.
-           ThreadBSSLocal,
+    /// ThreadBSSLocal - Zero-initialized TLS data objects with local linkage.
+    ThreadBSSLocal,
 
-       /// GlobalWriteableData - Writeable data that is global (not thread
-       /// local).
+    /// GlobalWriteableData - Writeable data that is global (not thread
+    /// local).
 
-           /// BSS - Zero initialized writeable data.
-           BSS,
+    /// BSS - Zero initialized writeable data.
+    BSS,
 
-               /// BSSLocal - This is BSS (zero initialized and writable) data
-               /// which has local linkage.
-               BSSLocal,
+    /// BSSLocal - This is BSS (zero initialized and writable) data
+    /// which has local linkage.
+    BSSLocal,
 
-               /// BSSExtern - This is BSS data with normal external linkage.
-               BSSExtern,
+    /// BSSExtern - This is BSS data with normal external linkage.
+    BSSExtern,
 
-           /// Common - Data with common linkage.  These represent tentative
-           /// definitions, which always have a zero initializer and are never
-           /// marked 'constant'.
-           Common,
+    /// Common - Data with common linkage.  These represent tentative
+    /// definitions, which always have a zero initializer and are never
+    /// marked 'constant'.
+    Common,
 
-           /// This is writeable data that has a non-zero initializer.
-           Data,
+    /// This is writeable data that has a non-zero initializer.
+    Data,
 
-           /// ReadOnlyWithRel - These are global variables that are never
-           /// written to by the program, but that have relocations, so they
-           /// must be stuck in a writeable section so that the dynamic linker
-           /// can write to them.  If it chooses to, the dynamic linker can
-           /// mark the pages these globals end up on as read-only after it is
-           /// done with its relocation phase.
-           ReadOnlyWithRel
+    /// ReadOnlyWithRel - These are global variables that are never
+    /// written to by the program, but that have relocations, so they
+    /// must be stuck in a writeable section so that the dynamic linker
+    /// can write to them.  If it chooses to, the dynamic linker can
+    /// mark the pages these globals end up on as read-only after it is
+    /// done with its relocation phase.
+    ReadOnlyWithRel
   } K : 8;
+
 public:
 
   bool isMetadata() const { return K == Metadata; }

@zahiraam zahiraam changed the title Shared section [CLANG] Add support of shared attribute for the pragma section. Mar 19, 2025
Copy link
Contributor

@tahonermann tahonermann left a comment

Choose a reason for hiding this comment

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

This code looks pretty clean; simpler than I thought would be required. Yay! I think I spotted why the section is getting labeled as constant; see comments.

Comment on lines 5697 to 5698
if ((SI.SectionFlags & ASTContext::PSF_Write) == 0)
GV->setConstant(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks to be where the constant specifier is coming from in the test case; absence of PSF_Write implies constant. Here is a suggestion, but there might be a better way to reorganize how these flags are checked; I haven't given it much thought.

Suggested change
if ((SI.SectionFlags & ASTContext::PSF_Write) == 0)
GV->setConstant(true);
if ((SI.SectionFlags & ASTContext::PSF_Write) == 0 &&
SI.SectionFlags & ASTContext::PSF_Shared) == 0)
GV->setConstant(true);

@zahiraam zahiraam closed this May 27, 2025
@zahiraam zahiraam deleted the SharedSection branch May 27, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants