llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

There are several large array declarations in llvm-libc. They usually look 
similar to this:
```c++
alignas(16) inline constexpr LogRR LOG_TABLE = {
    {
        {Sign::POS, 0, 0_u128},
        {Sign::POS, -134, 0x8080abac'46f38946'662d417c'ed007a46_u128},
        {Sign::POS, -133, 0x8102b2c4'9ac23a4f'91d082dc'e3ddcd38_u128},
        {Sign::POS, -133, 0xc2492946'4655f45c'da5f3cc0'b3251dbd_u128},
        {Sign::POS, -132, 0x820aec4f'3a222380'b9e3aea6'c444ef07_u128},
// ...
```
the `_u128` is a user-defined literal, so the hex constant to the left of it is 
actually a `StringLiteral` and the UDL converts that to a different type by 
iterating over all chars. It calls one function per char, and that function 
contains the usual switch statement over all ASCII characters.

This was problematic with the bytecode interpreter. Support for 
`-fconstexpr-steps` is implemented by counting the amount of jumps, but switch 
statements are implemented by comparing the switch condition to all case values 
and jumping.

The attached test case uses roughly 4'000 steps in the current interpreter but 
used to use over 8'000 with the bytecode interpreter. It now only uses 400 in 
the bytecode interpreter (which might be too low again but anyway).

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


8 Files Affected:

- (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.cpp (+4) 
- (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.h (+1) 
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+2-2) 
- (modified) clang/lib/AST/ByteCode/EvalEmitter.cpp (+4) 
- (modified) clang/lib/AST/ByteCode/EvalEmitter.h (+1) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+8-3) 
- (modified) clang/lib/AST/ByteCode/Opcodes.td (+2) 
- (added) clang/test/AST/ByteCode/jump-case.cpp (+140) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp 
b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
index 393b8481fecd1..4e0d7ea782a54 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
@@ -225,6 +225,10 @@ bool ByteCodeEmitter::jumpTrue(const LabelTy &Label, 
SourceInfo SI) {
   return emitJt(getOffset(Label), SI);
 }
 
+bool ByteCodeEmitter::jumpCase(const LabelTy &Label, SourceInfo SI) {
+  return emitJc(getOffset(Label), SI);
+}
+
 bool ByteCodeEmitter::jumpFalse(const LabelTy &Label, SourceInfo SI) {
   return emitJf(getOffset(Label), SI);
 }
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h 
b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 4b42b7eb4063b..8981a43f85b3f 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -55,6 +55,7 @@ class ByteCodeEmitter {
 
   /// Emits jumps.
   bool jumpTrue(const LabelTy &Label, SourceInfo SI);
+  bool jumpCase(const LabelTy &Label, SourceInfo SI);
   bool jumpFalse(const LabelTy &Label, SourceInfo SI);
   bool jump(const LabelTy &Label, SourceInfo SI);
   bool fallthrough(const LabelTy &Label);
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp 
b/clang/lib/AST/ByteCode/Compiler.cpp
index 6279b2fb5a38e..ed4de570f980c 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6654,7 +6654,7 @@ bool Compiler<Emitter>::visitSwitchStmt(const SwitchStmt 
*S) {
         PrimType HT = this->classifyPrim(High->getType());
         if (!this->emitLE(HT, S))
           return false;
-        if (!this->jumpTrue(CaseLabels[CS], S))
+        if (!this->jumpCase(CaseLabels[CS], S))
           return false;
         this->emitLabel(EndOfRangeCheck);
         continue;
@@ -6674,7 +6674,7 @@ bool Compiler<Emitter>::visitSwitchStmt(const SwitchStmt 
*S) {
       // Compare and jump to the case label.
       if (!this->emitEQ(ValueT, S))
         return false;
-      if (!this->jumpTrue(CaseLabels[CS], S))
+      if (!this->jumpCase(CaseLabels[CS], S))
         return false;
     } else {
       assert(!DefaultLabel);
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp 
b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index 3e1aade65afc8..87fed45646282 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -161,6 +161,10 @@ bool EvalEmitter::jumpTrue(const LabelTy &Label, 
SourceInfo SI) {
   return true;
 }
 
+bool EvalEmitter::jumpCase(const LabelTy &Label, SourceInfo SI) {
+  return jumpTrue(Label, SI);
+}
+
 bool EvalEmitter::jumpFalse(const LabelTy &Label, SourceInfo SI) {
   if (isActive()) {
     CurrentSource = SI;
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h 
b/clang/lib/AST/ByteCode/EvalEmitter.h
index 6fd50da8cad76..e355ea615ebea 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -73,6 +73,7 @@ class EvalEmitter : public SourceMapper {
 
   /// Emits jumps.
   bool jumpTrue(const LabelTy &Label, SourceInfo SI);
+  bool jumpCase(const LabelTy &Label, SourceInfo SI);
   bool jumpFalse(const LabelTy &Label, SourceInfo SI);
   bool jump(const LabelTy &Label, SourceInfo SI);
   bool fallthrough(const LabelTy &Label);
diff --git a/clang/lib/AST/ByteCode/Interp.cpp 
b/clang/lib/AST/ByteCode/Interp.cpp
index 699b034c3c683..b974ab75ebdf8 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -62,10 +62,15 @@ static bool Jmp(InterpState &S, CodePtr &PC, int32_t 
Offset) {
   return S.noteStep(PC);
 }
 
-static bool Jt(InterpState &S, CodePtr &PC, int32_t Offset) {
-  if (S.Stk.pop<bool>()) {
+/// jumpCase - like jumpTrue, but we do not note a step.
+static bool Jc(InterpState &S, CodePtr &PC, int32_t Offset) {
+  if (S.Stk.pop<bool>())
     PC += Offset;
-  }
+  return true;
+}
+
+static bool Jt(InterpState &S, CodePtr &PC, int32_t Offset) {
+  Jc(S, PC, Offset);
   return S.noteStep(PC);
 }
 
diff --git a/clang/lib/AST/ByteCode/Opcodes.td 
b/clang/lib/AST/ByteCode/Opcodes.td
index 4bd61cdce658d..b6274c13aa602 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -175,6 +175,8 @@ class JumpOpcode : Opcode {
 def Jmp : JumpOpcode;
 // [Bool] -> [], jumps if true.
 def Jt : JumpOpcode;
+// [Bool] -> [], jumps if true. Does NOT count as a step.
+def Jc : JumpOpcode;
 // [Bool] -> [], jumps if false.
 def Jf : JumpOpcode;
 
diff --git a/clang/test/AST/ByteCode/jump-case.cpp 
b/clang/test/AST/ByteCode/jump-case.cpp
new file mode 100644
index 0000000000000..1b4fcc328c2e9
--- /dev/null
+++ b/clang/test/AST/ByteCode/jump-case.cpp
@@ -0,0 +1,140 @@
+// RUN: %clang_cc1 -triple x86_64-linux -std=c++23 %s 
-fexperimental-new-constant-interpreter -fconstexpr-steps=5000
+
+constexpr int char_to_int(char ch) {
+  switch (ch) {
+  case '0':
+    return 0;
+  case '1':
+    return 1;
+  case '2':
+    return 2;
+  case '3':
+    return 3;
+  case '4':
+    return 4;
+  case '5':
+    return 5;
+  case '6':
+    return 6;
+  case '7':
+    return 7;
+  case '8':
+    return 8;
+  case '9':
+    return 9;
+  case 'a':
+    return 10;
+  case 'A':
+    return 10;
+  case 'b':
+    return 11;
+  case 'B':
+    return 11;
+  case 'c':
+    return 12;
+  case 'C':
+    return 12;
+  case 'd':
+    return 13;
+  case 'D':
+    return 13;
+  case 'e':
+    return 14;
+  case 'E':
+    return 14;
+  case 'f':
+    return 15;
+  case 'F':
+    return 15;
+  case 'g':
+    return 16;
+  case 'G':
+    return 16;
+  case 'h':
+    return 17;
+  case 'H':
+    return 17;
+  case 'i':
+    return 18;
+  case 'I':
+    return 18;
+  case 'j':
+    return 19;
+  case 'J':
+    return 19;
+  case 'k':
+    return 20;
+  case 'K':
+    return 20;
+  case 'l':
+    return 21;
+  case 'L':
+    return 21;
+  case 'm':
+    return 22;
+  case 'M':
+    return 22;
+  case 'n':
+    return 23;
+  case 'N':
+    return 23;
+  case 'o':
+    return 24;
+  case 'O':
+    return 24;
+  case 'p':
+    return 25;
+  case 'P':
+    return 25;
+  case 'q':
+    return 26;
+  case 'Q':
+    return 26;
+  case 'r':
+    return 27;
+  case 'R':
+    return 27;
+  case 's':
+    return 28;
+  case 'S':
+    return 28;
+  case 't':
+    return 29;
+  case 'T':
+    return 29;
+  case 'u':
+    return 30;
+  case 'U':
+    return 30;
+  case 'v':
+    return 31;
+  case 'V':
+    return 31;
+  case 'w':
+    return 32;
+  case 'W':
+    return 32;
+  case 'x':
+    return 33;
+  case 'X':
+    return 33;
+  case 'y':
+  case 'Y':
+    return 34;
+  case 'z':
+  case 'Z':
+    return 35;
+  default:
+    return 0;
+  }
+}
+constexpr bool check() {
+  const char *str = 
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+  unsigned sum = 0;
+  for (const char *p = str; *p != '\0'; ++p) {
+    sum+= char_to_int(*p);
+  }
+  return sum != 0;
+}
+
+static_assert(check());

``````````

</details>


https://github.com/llvm/llvm-project/pull/201845
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to