[clang] [llvm] [clang] Support per-function [[clang::code_align(N)]] attribute. (PR #80765)

2024-02-11 Thread Anton Bikineev via cfe-commits

AntonBikineev wrote:

> I am not sure a case where people care for specific functions and want 
> annotating them.

Our (V8 and Chromium) use case is that we know some hot functions that suffer 
from misaligned jumps. These functions would ideally be annotated with the 
attribute. We don't want to use the option for the entire binary as it'd 
significantly increase the binary size.

The alternative would be moving those functions into a separate TU(s), but it's 
just cumbersome.

https://github.com/llvm/llvm-project/pull/80765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang] Support per-function [[clang::code_align(N)]] attribute. (PR #80765)

2024-02-09 Thread Fangrui Song via cfe-commits

MaskRay wrote:

> > Aligning the targets of branches is a different thing from what you've 
> > implemented. There are basic blocks which are not branch targets, and there 
> > are branch targets which are not at the beginning of a basic block. (Branch 
> > targets that aren't at the beginning of a basic block are rare on most 
> > targets, but it doesn't seem like something we should completely ignore.)
> 
> This makes sense, thanks. Would there be an interest in something like 
> [[clang::x86_align_branch_boundary]] that would align branch instructions in 
> a function?

(I was involved in adding `-mbranches-within-32B-boundaries` and relevant 
assembler support.) I think the interest is low. A function attribute is for 
finer granularity so that you can mix some functions with unannotated 
functions. Users either specify `-mbranches-within-32B-boundaries` to mitigate 
performance penalty for the whole TU (or globally for LTO) when a microcode 
update is applied for certain Intel CPUs or not. I am not sure a case where 
people care for specific functions and want annotating them.

https://github.com/llvm/llvm-project/pull/80765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang] Support per-function [[clang::code_align(N)]] attribute. (PR #80765)

2024-02-09 Thread Anton Bikineev via cfe-commits

AntonBikineev wrote:

> Aligning the targets of branches is a different thing from what you've 
> implemented. There are basic blocks which are not branch targets, and there 
> are branch targets which are not at the beginning of a basic block. (Branch 
> targets that aren't at the beginning of a basic block are rare on most 
> targets, but it doesn't seem like something we should completely ignore.)

That makes sense, thanks. Would there be an interest in something like 
[[clang::x86_align_branch_boundary]] that would align branch instructions per 
function?

https://github.com/llvm/llvm-project/pull/80765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang] Support per-function [[clang::code_align(N)]] attribute. (PR #80765)

2024-02-06 Thread Eli Friedman via cfe-commits

efriedma-quic wrote:

> > I'm skeptical this actually makes sense in its current form. LLVM 
> > internally has a thing it calls a basic block, but what exactly counts is, 
> > to some extent, machine-specific, and can change from version to version.
> 
> I guess regardless of whether we call it BB alignment or label alignment, the 
> concept is already supported on the frontend side for loops. My idea was that 
> we could lift it to the function level, forcing all the function "labels" be 
> aligned on a certain boundary.

Aligning the targets of branches is a different thing from what you've 
implemented.  There are basic blocks which are not branch targets, and there 
are branch targets which are not at the beginning of a basic block.  (Branch 
targets that aren't at the beginning of a basic block are rare on most targets, 
but it doesn't seem like something we should completely ignore.)

"Loop alignment" is specifically a request to override the normal compiler 
heuristics for aligning the loop header i.e. the target of the loop backedges.  
This is best-effort; a "loop" in C code might not lower to something that 
actually has a header we can align.

"-malign-branch-boundary" specifically inserts padding so branch instructions 
don't cross a 32-byte boundary; it isn't trying to align anything.

https://github.com/llvm/llvm-project/pull/80765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang] Support per-function [[clang::code_align(N)]] attribute. (PR #80765)

2024-02-05 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-backend-x86

Author: Anton Bikineev (AntonBikineev)


Changes

The existing attribute works only for loop headers. This can unfortunately be 
quite limiting, especially as a protection against the "Jump Conditional Code 
Erratum" [0] (for example, if there is an unaligned check in a hot loop). The 
command line option -malign-branch-boundary can help with that, but it's too 
coarse-grained and can significantly increase the binary size.

This change introduces a per-function [[clang::code_align(N)]] attribute that 
aligns all the basic blocks of a function by the given N.

[0] 
https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf

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


8 Files Affected:

- (modified) clang/include/clang/Basic/Attr.td (+4-3) 
- (modified) clang/include/clang/Basic/AttrDocs.td (+11-5) 
- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+6) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+10) 
- (added) clang/test/CodeGen/code_align_function.c (+42) 
- (modified) clang/test/Sema/code_align.c (+12-2) 
- (modified) llvm/lib/CodeGen/MachineBlockPlacement.cpp (+21-6) 
- (added) llvm/test/CodeGen/X86/code-align-basic-blocks.ll (+29) 


``diff
diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index b2d5309e142c1..abce685e9f7a6 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4435,10 +4435,11 @@ def PreferredType: InheritableAttr {
   let Documentation = [PreferredTypeDocumentation];
 }
 
-def CodeAlign: StmtAttr {
+def CodeAlign : InheritableAttr {
   let Spellings = [Clang<"code_align">];
-  let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt],
-  ErrorDiag, "'for', 'while', and 'do' 
statements">;
+  let Subjects =
+  SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt, Function],
+  ErrorDiag, "'for', 'while', 'do' statements and functions">;
   let Args = [ExprArgument<"Alignment">];
   let Documentation = [CodeAlignAttrDocs];
   let AdditionalMembers = [{
diff --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 041786f37fb8a..57b63469f1573 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7704,11 +7704,12 @@ def CodeAlignAttrDocs : Documentation {
   let Category = DocCatVariable;
   let Heading = "clang::code_align";
   let Content = [{
-The ``clang::code_align(N)`` attribute applies to a loop and specifies the byte
-alignment for a loop. The attribute accepts a positive integer constant
-initialization expression indicating the number of bytes for the minimum
-alignment boundary. Its value must be a power of 2, between 1 and 4096
-(inclusive).
+The ``clang::code_align(N)`` attribute applies to a loop or a function. When
+applied to a loop it specifies the byte alignment for the loop header. When
+applied to a function it aligns all the basic blocks of the function. The
+attribute accepts a positive integer constant initialization expression
+indicating the number of bytes for the minimum alignment boundary. Its value
+must be a power of 2, between 1 and 4096 (inclusive).
 
 .. code-block:: c++
 
@@ -7738,6 +7739,11 @@ alignment boundary. Its value must be a power of 2, 
between 1 and 4096
 [[clang::code_align(A)]] for(;;) { }
   }
 
+  [[clang:code_align(16)]] int foo(bool b) {
+if (b) return 2;
+return 3;
+  }
+
   }];
 }
 
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 36b63d78b06f8..87a822e91fff6 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2515,6 +2515,12 @@ void 
CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 
+  if (const auto *CodeAlign = D->getAttr()) {
+const auto *CE = cast(CodeAlign->getAlignment());
+std::string ArgValStr = llvm::toString(CE->getResultAsAPSInt(), 10);
+F->addFnAttr("align-basic-blocks", ArgValStr);
+  }
+
   // In the cross-dso CFI mode with canonical jump tables, we want !type
   // attributes on definitions only.
   if (CodeGenOpts.SanitizeCfiCrossDso &&
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d785714c4d811..40412801b632a 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -9036,6 +9036,12 @@ static void handleArmNewAttr(Sema &S, Decl *D, const 
ParsedAttr &AL) {
  ArmNewAttr(S.Context, AL, NewState.data(), NewState.size()));
 }
 
+static void handleCodeAlignAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  Expr *E = A.getArgAsExpr(0);
+  if (Attr *CodeAlignAttr = S.BuildCodeAlignAttr(A, E))
+D->addAttr(CodeAlignAttr);
+}
+
 /// ProcessDeclAttribute - Apply the specific