[clang] [clang][CodeGen] Fix in codegen for __builtin_popcountg/ctzg/clzg (PR #90845)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Fix in codegen for __builtin_popcountg/ctzg/clzg (PR #90845)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/90845

From 15b41263850f4428c3a273f404c9c3543985e47d Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 2 May 2024 12:42:00 +0200
Subject: [PATCH] [clang][CodeGen] Fix in codegen for
 __builtin_popcountg/ctzg/clzg

Make sure that the result from the popcnt/ctlz/cttz intrinsics is
unsigned casted to int, rather than casted as a signed value, when
expanding the __builtin_popcountg/__builtin_ctzg/__builtin_clzg
builtins.

An example would be
  unsigned _BitInt(1) x = ...;
  int y = __builtin_popcountg(x);
which previously was incorrectly expanded to
  %1 = call i1 @llvm.ctpop.i1(i1 %0)
  %cast = sext i1 %1 to i32

Since the input type is generic for those "g" versions of the builtins
the intrinsic call may return a value for which the sign bit is set
(that could typically for BitInt of size 1 and 2). So we need to
emit a zext rather than a sext to avoid negative results.
---
 clang/lib/CodeGen/CGBuiltin.cpp  |  12 +--
 clang/test/CodeGen/builtins-bitint.c | 126 +++
 clang/test/CodeGen/builtins.c|  20 ++---
 3 files changed, 142 insertions(+), 16 deletions(-)
 create mode 100644 clang/test/CodeGen/builtins-bitint.c

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a370734e00d3e1..fdc060c05b72fe 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3239,8 +3239,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
- "cast");
+  Result =
+  Builder.CreateIntCast(Result, ResultType, /*isSigned*/ false, 
"cast");
 if (!HasFallback)
   return RValue::get(Result);
 
@@ -3271,8 +3271,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
- "cast");
+  Result =
+  Builder.CreateIntCast(Result, ResultType, /*isSigned*/ false, 
"cast");
 if (!HasFallback)
   return RValue::get(Result);
 
@@ -3351,8 +3351,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 llvm::Type *ResultType = ConvertType(E->getType());
 Value *Result = Builder.CreateCall(F, ArgValue);
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
- "cast");
+  Result =
+  Builder.CreateIntCast(Result, ResultType, /*isSigned*/ false, 
"cast");
 return RValue::get(Result);
   }
   case Builtin::BI__builtin_unpredictable: {
diff --git a/clang/test/CodeGen/builtins-bitint.c 
b/clang/test/CodeGen/builtins-bitint.c
new file mode 100644
index 00..804e4971287737
--- /dev/null
+++ b/clang/test/CodeGen/builtins-bitint.c
@@ -0,0 +1,126 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 4
+// RUN: %clang_cc1 -triple arm-unknown-unknown -O0 -std=c23 -emit-llvm %s -o - 
| FileCheck %s --check-prefix=CHECK-O0
+// RUN: %clang_cc1 -triple arm-unknown-unknown -O1 -std=c23 -emit-llvm %s -o - 
| FileCheck %s --check-prefix=CHECK-O1
+
+// Verify that the result from the intrinsic call is zero extended to avoid 
that
+// we get a negative result from popcountg/ctzg/clzg.
+
+// CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi1(
+// CHECK-O0-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-O0-NEXT:  entry:
+// CHECK-O0-NEXT:[[A:%.*]] = alloca i1, align 1
+// CHECK-O0-NEXT:store i1 true, ptr [[A]], align 1
+// CHECK-O0-NEXT:[[TMP0:%.*]] = load i1, ptr [[A]], align 1
+// CHECK-O0-NEXT:[[TMP1:%.*]] = call i1 @llvm.ctpop.i1(i1 [[TMP0]])
+// CHECK-O0-NEXT:[[CAST:%.*]] = zext i1 [[TMP1]] to i32
+// CHECK-O0-NEXT:ret i32 [[CAST]]
+//
+// CHECK-O1-LABEL: define dso_local arm_aapcscc noundef i32 
@test_popcountg_ubi1(
+// CHECK-O1-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-O1-NEXT:  entry:
+// CHECK-O1-NEXT:ret i32 1
+//
+int test_popcountg_ubi1() {
+  unsigned _BitInt(1) a = 1uwb;
+  return __builtin_popcountg(a);
+}
+
+// CHECK-O0-LABEL: define dso_local arm_aapcscc i32 @test_popcountg_ubi2(
+// CHECK-O0-SAME: ) #[[ATTR0]] {
+// CHECK-O0-NEXT:  entry:
+// CHECK-O0-NEXT:[[A:%.*]] = alloca i2, align 1
+// CHECK-O0-NEXT:store i2 -1, ptr [[A]], align 1
+// CHECK-O0-NEXT:[[TMP0:%.*]] = load i2, ptr [[A]], align 1
+// CHECK-O0-NEXT:

[clang] [clang][CodeGen] Fix in codegen for __builtin_popcountg/ctzg/clzg (PR #90845)

[Björn Pettersson via cfe-commits]

bjope wrote:

> Please add a testcase for i1/i2 (which are the only cases where this actually 
> matters).

I added tests in a new file (builtins-bitint.c) to show that we get expected 
results for a target that isn't treating a zero input to ctzg/clzg as poison.

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

[clang] [clang][CodeGen] Fix in codegen for __builtin_popcountg/ctzg/clzg (PR #90845)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/90845

From bab0da08c41ed13935ad607e633939b6870d4ffb Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 2 May 2024 12:42:00 +0200
Subject: [PATCH 1/2] [clang][CodeGen] Fix in codegen for
 __builtin_popcountg/ctzg/clzg

Make sure that the result from the popcnt/ctlz/cttz intrinsics is
unsigned casted to int, rather than casted as a signed value, when
expanding the __builtin_popcountg/__builtin_ctzg/__builtin_clzg
builtins.

An example would be
  unsigned _BitInt(1) x = ...;
  int y = __builtin_popcountg(x);
which previously was incorrectly expanded to
  %1 = call i1 @llvm.ctpop.i1(i1 %0)
  %cast = sext i1 %1 to i32

Since the input type is generic for those "g" versions of the builtins
the intrinsic call may return a value for which the sign bit is set
(that could typically for BitInt of size 1 and 2). So we need to
emit a zext rather than a sext to avoid negative results.
---
 clang/lib/CodeGen/CGBuiltin.cpp |  6 +++---
 clang/test/CodeGen/builtins.c   | 20 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a370734e00d3e1..fc1dbb87ed1bf2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3239,7 +3239,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 if (!HasFallback)
   return RValue::get(Result);
@@ -3271,7 +3271,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 if (!HasFallback)
   return RValue::get(Result);
@@ -3351,7 +3351,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 llvm::Type *ResultType = ConvertType(E->getType());
 Value *Result = Builder.CreateCall(F, ArgValue);
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 return RValue::get(Result);
   }
diff --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index 407e0857d22311..b41efb59e61dbc 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -949,12 +949,12 @@ void test_builtin_popcountg(unsigned char uc, unsigned 
short us,
   pop = __builtin_popcountg(uc);
   // CHECK: %1 = load i8, ptr %uc.addr, align 1
   // CHECK-NEXT: %2 = call i8 @llvm.ctpop.i8(i8 %1)
-  // CHECK-NEXT: %cast = sext i8 %2 to i32
+  // CHECK-NEXT: %cast = zext i8 %2 to i32
   // CHECK-NEXT: store volatile i32 %cast, ptr %pop, align 4
   pop = __builtin_popcountg(us);
   // CHECK-NEXT: %3 = load i16, ptr %us.addr, align 2
   // CHECK-NEXT: %4 = call i16 @llvm.ctpop.i16(i16 %3)
-  // CHECK-NEXT: %cast1 = sext i16 %4 to i32
+  // CHECK-NEXT: %cast1 = zext i16 %4 to i32
   // CHECK-NEXT: store volatile i32 %cast1, ptr %pop, align 4
   pop = __builtin_popcountg(ui);
   // CHECK-NEXT: %5 = load i32, ptr %ui.addr, align 4
@@ -992,12 +992,12 @@ void test_builtin_clzg(unsigned char uc, unsigned short 
us, unsigned int ui,
   lz = __builtin_clzg(uc);
   // CHECK: %1 = load i8, ptr %uc.addr, align 1
   // CHECK-NEXT: %2 = call i8 @llvm.ctlz.i8(i8 %1, i1 true)
-  // CHECK-NEXT: %cast = sext i8 %2 to i32
+  // CHECK-NEXT: %cast = zext i8 %2 to i32
   // CHECK-NEXT: store volatile i32 %cast, ptr %lz, align 4
   lz = __builtin_clzg(us);
   // CHECK-NEXT: %3 = load i16, ptr %us.addr, align 2
   // CHECK-NEXT: %4 = call i16 @llvm.ctlz.i16(i16 %3, i1 true)
-  // CHECK-NEXT: %cast1 = sext i16 %4 to i32
+  // CHECK-NEXT: %cast1 = zext i16 %4 to i32
   // CHECK-NEXT: store volatile i32 %cast1, ptr %lz, align 4
   lz = __builtin_clzg(ui);
   // CHECK-NEXT: %5 = load i32, ptr %ui.addr, align 4
@@ -1026,7 +1026,7 @@ void test_builtin_clzg(unsigned char uc, unsigned short 
us, unsigned int ui,
   lz = __builtin_clzg(uc, sc);
   // CHECK-NEXT: %15 = load i8, ptr %uc.addr, align 1
   // CHECK-NEXT: %16 = call i8 @llvm.ctlz.i8(i8 %15, i1 true)
-  // CHECK-NEXT: %cast6 = sext i8 %16 to i32
+  // CHECK-NEXT: %cast6 = zext i8 %16 to i32
   // 

[clang] [clang] Implement __builtin_popcountg (PR #82359)

[Björn Pettersson via cfe-commits]

@@ -3216,7 +3216,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__popcnt64:
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
-  case Builtin::BI__builtin_popcountll: {
+  case Builtin::BI__builtin_popcountll:
+  case Builtin::BI__builtin_popcountg: {

bjope wrote:

I've opened a pull-request to also adjust the codegen here, to make sure that 
we use an unsigned cast between the LLVM intrinsic result type and the builtins 
return type. See https://github.com/llvm/llvm-project/pull/90845

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

[clang] [clang][CodeGen] Fix in codegen for __builtin_popcountg/ctzg/clzg (PR #90845)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/90845

Make sure that the result from the popcnt/ctlz/cttz intrinsics is unsigned 
casted to int, rather than casted as a signed value, when expanding the 
__builtin_popcountg/__builtin_ctzg/__builtin_clzg builtins.

An example would be
  unsigned _BitInt(1) x = ...;
  int y = __builtin_popcountg(x);
which previously was incorrectly expanded to
  %1 = call i1 @llvm.ctpop.i1(i1 %0)
  %cast = sext i1 %1 to i32

Since the input type is generic for those "g" versions of the builtins the 
intrinsic call may return a value for which the sign bit is set (that could 
typically for BitInt of size 1 and 2). So we need to emit a zext rather than a 
sext to avoid negative results.

From bab0da08c41ed13935ad607e633939b6870d4ffb Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 2 May 2024 12:42:00 +0200
Subject: [PATCH] [clang][CodeGen] Fix in codegen for
 __builtin_popcountg/ctzg/clzg

Make sure that the result from the popcnt/ctlz/cttz intrinsics is
unsigned casted to int, rather than casted as a signed value, when
expanding the __builtin_popcountg/__builtin_ctzg/__builtin_clzg
builtins.

An example would be
  unsigned _BitInt(1) x = ...;
  int y = __builtin_popcountg(x);
which previously was incorrectly expanded to
  %1 = call i1 @llvm.ctpop.i1(i1 %0)
  %cast = sext i1 %1 to i32

Since the input type is generic for those "g" versions of the builtins
the intrinsic call may return a value for which the sign bit is set
(that could typically for BitInt of size 1 and 2). So we need to
emit a zext rather than a sext to avoid negative results.
---
 clang/lib/CodeGen/CGBuiltin.cpp |  6 +++---
 clang/test/CodeGen/builtins.c   | 20 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a370734e00d3e1..fc1dbb87ed1bf2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3239,7 +3239,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 if (!HasFallback)
   return RValue::get(Result);
@@ -3271,7 +3271,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.getInt1(HasFallback || getTarget().isCLZForZeroUndef());
 Value *Result = Builder.CreateCall(F, {ArgValue, ZeroUndef});
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 if (!HasFallback)
   return RValue::get(Result);
@@ -3351,7 +3351,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 llvm::Type *ResultType = ConvertType(E->getType());
 Value *Result = Builder.CreateCall(F, ArgValue);
 if (Result->getType() != ResultType)
-  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/false,
  "cast");
 return RValue::get(Result);
   }
diff --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index 407e0857d22311..b41efb59e61dbc 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -949,12 +949,12 @@ void test_builtin_popcountg(unsigned char uc, unsigned 
short us,
   pop = __builtin_popcountg(uc);
   // CHECK: %1 = load i8, ptr %uc.addr, align 1
   // CHECK-NEXT: %2 = call i8 @llvm.ctpop.i8(i8 %1)
-  // CHECK-NEXT: %cast = sext i8 %2 to i32
+  // CHECK-NEXT: %cast = zext i8 %2 to i32
   // CHECK-NEXT: store volatile i32 %cast, ptr %pop, align 4
   pop = __builtin_popcountg(us);
   // CHECK-NEXT: %3 = load i16, ptr %us.addr, align 2
   // CHECK-NEXT: %4 = call i16 @llvm.ctpop.i16(i16 %3)
-  // CHECK-NEXT: %cast1 = sext i16 %4 to i32
+  // CHECK-NEXT: %cast1 = zext i16 %4 to i32
   // CHECK-NEXT: store volatile i32 %cast1, ptr %pop, align 4
   pop = __builtin_popcountg(ui);
   // CHECK-NEXT: %5 = load i32, ptr %ui.addr, align 4
@@ -992,12 +992,12 @@ void test_builtin_clzg(unsigned char uc, unsigned short 
us, unsigned int ui,
   lz = __builtin_clzg(uc);
   // CHECK: %1 = load i8, ptr %uc.addr, align 1
   // CHECK-NEXT: %2 = call i8 @llvm.ctlz.i8(i8 %1, i1 true)
-  // CHECK-NEXT: %cast = sext i8 %2 to i32
+  // CHECK-NEXT: %cast = zext i8 %2 to i32
   // CHECK-NEXT: store volatile i32 %cast, ptr %lz, align 4
   lz = __builtin_clzg(us);
   // CHECK-NEXT: %3 = load i16, ptr %us.addr, 

[clang] [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (PR #88004)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (PR #88004)

[Björn Pettersson via cfe-commits]

@@ -146,6 +146,15 @@ struct BinOpInfo {
   return UnOp->getSubExpr()->getType()->isFixedPointType();
 return false;
   }
+
+  /// Check if the RHS has a signed integer representation.

bjope wrote:

This is a Doxygen style comment (like the ones for other functions above). So 
there should be three '/'.

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

[clang] [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (PR #88004)

[Björn Pettersson via cfe-commits]

bjope wrote:

ping

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

[clang] [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (PR #88004)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/88004

Commit 5f87957fefb21d454f2f (pull-requst #80515) corrected some codegen 
problems related to _BitInt types being used as shift exponents. But it did not 
fix it properly for the special case when the shift count operand is a signed 
_BitInt.

The basic problem is the same as the one solved for unsigned _BitInt. As we use 
an unsigned comparison to see if the shift exponent is out-of-bounds, then we 
need to find an unsigned maximum allowed shift amount to use in the check. 
Normally the shift amount is limited by bitwidth of the LHS of the shift. 
However, when the RHS type is small in relation to the LHS then we need to use 
a value that fits inside the bitwidth of the RHS instead.

The earlier fix simply used the unsigned maximum when deterining the max shift 
amount based on the RHS type. It did however not take into consideration that 
the RHS type could have a signed representation. In such situations we need to 
use the signed maximum instead. Otherwise we do not recognize a negative shift 
exponent as UB.

From df9c1202ec1e9ceb7c1079201b94bae8576cd73f Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 8 Apr 2024 14:33:07 +0200
Subject: [PATCH] [clang][CodeGen] Fix shift-exponent ubsan check for signed
 _BitInt

Commit 5f87957fefb21d454f2f (pull-requst #80515) corrected some
codegen problems related to _BitInt types being used as shift
exponents. But it did not fix it properly for the special
case when the shift count operand is a signed _BitInt.

The basic problem is the same as the one solved for unsigned
_BitInt. As we use an unsigned comparison to see if the shift
exponent is out-of-bounds, then we need to find an unsigned
maximum allowed shift amount to use in the check. Normally the
shift amount is limited by bitwidth of the LHS of the shift.
However, when the RHS type is small in relation to the LHS then
we need to use a value that fits inside the bitwidth of the RHS
instead.

The earlier fix simply used the unsigned maximum when deterining
the max shift amount based on the RHS type. It did however not
take into consideration that the RHS type could have a signed
representation. In such situations we need to use the signed
maximum instead. Otherwise we do not recognize a negative shift
exponent as UB.
---
 clang/lib/CodeGen/CGExprScalar.cpp  | 31 +--
 clang/test/CodeGen/ubsan-shift-bitint.c | 33 +++--
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 397b4977acc3e9..12721c9ac0d26e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -146,6 +146,15 @@ struct BinOpInfo {
   return UnOp->getSubExpr()->getType()->isFixedPointType();
 return false;
   }
+
+  /// Check if the RHS has a signed integer representation.
+  bool rhsHasSignedIntegerRepresentation() const {
+if (const auto *BinOp = dyn_cast(E)) {
+  QualType RHSType = BinOp->getRHS()->getType();
+  return RHSType->hasSignedIntegerRepresentation();
+}
+return false;
+  }
 };
 
 static bool MustVisitNullValue(const Expr *E) {
@@ -780,7 +789,7 @@ class ScalarExprEmitter
   void EmitUndefinedBehaviorIntegerDivAndRemCheck(const BinOpInfo ,
   llvm::Value *Zero,bool 
isDiv);
   // Common helper for getting how wide LHS of shift is.
-  static Value *GetMaximumShiftAmount(Value *LHS, Value *RHS);
+  static Value *GetMaximumShiftAmount(Value *LHS, Value *RHS, bool 
RHSIsSigned);
 
   // Used for shifting constraints for OpenCL, do mask for powers of 2, URem 
for
   // non powers of two.
@@ -4181,7 +4190,8 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo ) {
   return Builder.CreateExactSDiv(diffInChars, divisor, "sub.ptr.div");
 }
 
-Value *ScalarExprEmitter::GetMaximumShiftAmount(Value *LHS, Value *RHS) {
+Value *ScalarExprEmitter::GetMaximumShiftAmount(Value *LHS, Value *RHS,
+bool RHSIsSigned) {
   llvm::IntegerType *Ty;
   if (llvm::VectorType *VT = dyn_cast(LHS->getType()))
 Ty = cast(VT->getElementType());
@@ -4192,7 +4202,9 @@ Value *ScalarExprEmitter::GetMaximumShiftAmount(Value 
*LHS, Value *RHS) {
   // this in ConstantInt::get, this results in the value getting truncated.
   // Constrain the return value to be max(RHS) in this case.
   llvm::Type *RHSTy = RHS->getType();
-  llvm::APInt RHSMax = llvm::APInt::getMaxValue(RHSTy->getScalarSizeInBits());
+  llvm::APInt RHSMax =
+  RHSIsSigned ? 
llvm::APInt::getSignedMaxValue(RHSTy->getScalarSizeInBits())
+  : llvm::APInt::getMaxValue(RHSTy->getScalarSizeInBits());
   if (RHSMax.ult(Ty->getBitWidth()))
 return llvm::ConstantInt::get(RHSTy, RHSMax);
   return llvm::ConstantInt::get(RHSTy, Ty->getBitWidth() - 1);
@@ -4207,7 +4219,7 @@ Value 

[clang] [OpenMP] Fix test after updating library search paths (PR #83573)

[Björn Pettersson via cfe-commits]

https://github.com/bjope approved this pull request.


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

[clang] [OpenMP] Fix test after updating library search paths (PR #83573)

[Björn Pettersson via cfe-commits]

@@ -101,17 +101,6 @@
 
 /// ###
 
-/// Check that the warning is thrown when the libomptarget bitcode library is 
not found.
-/// Libomptarget requires sm_52 or newer so an sm_52 bitcode library should 
never exist.
-// RUN:   not %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
\
-// RUN:   -Xopenmp-target -march=sm_52 
--cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-BCLIB-WARN %s
-
-// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_52.bc' found in the 
default clang lib directory or in LIBRARY_PATH; use 
'--libomptarget-nvptx-bc-path' to specify nvptx bitcode library

bjope wrote:

Ok. Just wanted to raise the concern. Anyway this patch would help, to avoid 
failures in our downstream buildbots (and I wouldn't need to workaround those 
problems downstream).
So LGTM!

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

[clang] [OpenMP] Fix test after updating library search paths (PR #83573)

[Björn Pettersson via cfe-commits]

@@ -101,17 +101,6 @@
 
 /// ###
 
-/// Check that the warning is thrown when the libomptarget bitcode library is 
not found.
-/// Libomptarget requires sm_52 or newer so an sm_52 bitcode library should 
never exist.
-// RUN:   not %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
\
-// RUN:   -Xopenmp-target -march=sm_52 
--cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-BCLIB-WARN %s
-
-// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_52.bc' found in the 
default clang lib directory or in LIBRARY_PATH; use 
'--libomptarget-nvptx-bc-path' to specify nvptx bitcode library

bjope wrote:

When removing this test you suddenly lose code coverage for the 
err_drv_omp_offload_target_missingbcruntime diagnostic. That seems a bit 
unfortunate.

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

[clang] [openmp] [OpenMP] Respect LLVM per-target install directories (PR #83282)

[Björn Pettersson via cfe-commits]

bjope wrote:

Problem 1 can be solved by flipping the order.
But Problem 2 would remain as it doesn't depend on the order.

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

[clang] [openmp] [OpenMP] Respect LLVM per-target install directories (PR #83282)

[Björn Pettersson via cfe-commits]

bjope wrote:

Hi @jhuber6, @MaskRay 

We are having some problems with this patch on a server where the file 
/lib64/libomptarget-nvptx-sm_52.bc exists.
The test case that fails is clang/test/Driver/openmp-offload-gpu.c.

**Problem 1**
I think one problem is related to this check line
`// CHK-ENV-BCLIB: 
clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}subdir{{/|}}libomptarget-nvptx-sm_52.bc
`
That test is using `env LIBRARY_PATH` but your changes in 
`tools::addOpenMPDeviceRTL` makes it prioritize the standard library paths 
before the environment. Not sure if that is how it should be or if env should 
have higher prio (i.e. added to LibraryPaths before the paths found in HostTC).

**Problem 2**
This check line also started failing:
`// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_52.bc' found in the 
default clang lib directory or in LIBRARY_PATH; use 
'--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
`

Now, with your path, I guess it starts picking up the 
`/lib64/libomptarget-nvptx-sm_52.bc` file from the system. So we no longer get 
the warning. Is that the intention with your patch? Regardless, I think you 
need to do something with that test case because I think the "should never 
exist" part in
```
/// Check that the warning is thrown when the libomptarget bitcode library is 
not found.
/// Libomptarget requires sm_52 or newer so an sm_52 bitcode library should 
never exist.
```
no longer holds with your patch.

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

[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

[Björn Pettersson via cfe-commits]

https://github.com/bjope approved this pull request.

LG

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

[clang] [Clang] Fix crash when recovering from an invalid pack indexing type. (PR #80652)

[Björn Pettersson via cfe-commits]

bjope wrote:

@cor3ntin : I've verified that this would solve the problem that I noticed 
downstream. Thanks!

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

[clang] [Clang] Fix a crash when dumping a pack indexing type. (PR #80439)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -std=c++2c -ast-dump %s | FileCheck  %s
+
+namespace InvalidPacksShouldNotCrash {
+
+struct NotAPack;
+template  typename Tp>
+void not_pack() {
+int i = 0;
+i...[0]; // expected-error {{i does not refer to the name of a parameter 
pack}}
+V...[0]; // expected-error {{V does not refer to the name of a parameter 
pack}}
+NotAPack...[0] a; // expected-error{{'NotAPack' does not refer to the name 
of a parameter pack}}

bjope wrote:

If you change this to an array, such as

`NotAPack...[0] a[2];`

then I think you hit the same kind of infinite recursion as we see downstream 
even without the array nodtation.

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

[clang] [Clang] Fix a crash when dumping a pack indexing type. (PR #80439)

[Björn Pettersson via cfe-commits]

bjope wrote:

@cor3ntin , our downstream code is doing some extra semantic checks using code 
like this in Sema::CheckVariableDeclarationType:

```
  if (!T->isIncompleteType() && !isDependentOrGNUAutoType(T) && 
!T->isPlaceholderType()) {
uint64_t Size = Context.getTypeSizeInChars(T).getQuantity();
   ...
  }
```
That is hitting the infinite recursion.

So we protect the type size calculation by checking if the type is incomplete, 
dependent, etc.
But as Bevin pointed out here 
https://github.com/llvm/llvm-project/pull/72644/files#r1469490392 the "broken" 
PackIndexingType isn't reported as isDependent, and neither as isIncomplete.

I must say that I don't know that much about this to say what is correct. Maybe 
out downstream semantic checks should be protected in some more way to avoid 
this problem.

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

[clang] [Clang] Fix a crash when dumping a pack indexing type. (PR #80439)

[Björn Pettersson via cfe-commits]

bjope wrote:

Seems like this didn't really solve the problem reported by @bevin-hansson (at 
least not completely).
The new test case fails for us:

```
FAIL: Clang :: AST/ast-dump-pack-indexing-crash.cpp (401 of 78930)
 TEST 'Clang :: AST/ast-dump-pack-indexing-crash.cpp' 
FAILED 
Exit Code: 2

Command Output (stderr):
--
RUN: at line 1: not /build/bin/clang -cc1 -internal-isystem 
/build/lib/clang/19/include -nostdsysteminc -std=c++2c -ast-dump 
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp | 
/build/bin/FileCheck  
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp
+ not /build/bin/clang -cc1 -internal-isystem /build/lib/clang/19/include 
-nostdsysteminc -std=c++2c -ast-dump 
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp
+ /build/bin/FileCheck 
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:9:5: error: i 
does not refer to the name of a parameter pack
9 | i...[0]; // expected-error {{i does not refer to the name of a 
parameter pack}}
  | ^
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:10:5: error: V 
does not refer to the name of a parameter pack
   10 | V...[0]; // expected-error {{V does not refer to the name of a 
parameter pack}}
  | ^
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:11:5: error: 
'NotAPack' does not refer to the name of a parameter pack
   11 | NotAPack...[0] a; // expected-error{{'NotAPack' does not refer to 
the name of a parameter pack}}
  | ^
Stack dump:
0.  Program arguments: /build/bin/clang -cc1 -internal-isystem 
/build/lib/clang/19/include -nostdsysteminc -std=c++2c -ast-dump 
/llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp
1.  /llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:11:21: 
current parser token ';'
2.  /llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:3:1: 
parsing namespace 'InvalidPacksShouldNotCrash'
3.  /llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:7:17: 
parsing function body 'InvalidPacksShouldNotCrash::not_pack'
4.  /llvm-project/clang/test/AST/ast-dump-pack-indexing-crash.cpp:7:17: in 
compound statement ('{}')
  #0 0x03b147c8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/build/bin/clang+0x3b147c8)
  #1 0x03b11f1c SignalHandler(int) Signals.cpp:0:0
  #2 0x7f49db4aa630 __restore_rt sigaction.c:0:0
  #3 0x071e19f0 clang::PackIndexingType::getSelectedIndex() const 
(.part.0) Type.cpp:0:0
  #4 0x06c9e257 clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e257)
  #5 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
  #6 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
  #7 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
  #8 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
  #9 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #10 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #11 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #12 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #13 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #14 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #15 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #16 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #17 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #18 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #19 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #20 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #21 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #22 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 #23 0x06c86ca7 clang::ASTContext::getTypeInfo(clang::Type const*) 
const (/build/bin/clang+0x6c86ca7)
 #24 0x06c9e21b clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const (/build/bin/clang+0x6c9e21b)
 

[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 %s -O0 -fsanitize=shift-exponent -emit-llvm -o - | 
FileCheck %s
+
+// Checking that the code generation is using the unextended/untruncated
+// exponent values and capping the values accordingly
+
+// CHECK-LABEL: define{{.*}} i32 @test_left_variable
+int test_left_variable(unsigned _BitInt(5) b, unsigned _BitInt(2) e) {
+  // CHECK: [[E_REG:%.+]] = load [[E_SIZE:i2]]
+  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], -1

bjope wrote:

So this test case show something that we perhaps should follow up an optimize 
later.
Not sure if the idea is to make the frontend simple at let the middle end 
optimize it away, but it is a bit stupid to emit the check here when comparing 
with -1.

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

[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

[Björn Pettersson via cfe-commits]

@@ -4121,6 +4121,13 @@ Value *ScalarExprEmitter::GetWidthMinusOneValue(Value* 
LHS,Value* RHS) {
 Ty = cast(VT->getElementType());
   else
 Ty = cast(LHS->getType());
+  // Testing with small _BitInt types has shown that Ty->getBitwidth() - 1

bjope wrote:

Prefer if the code comment just help describe what it is doing, and possibly 
why here.
IMO, the "Testing with small _BitInt types has shown..." part is not really 
that interesting as a code comment.

One idea is to rename the `GetWidthMinusOneValue` helper function as we 
slightly change what it does.
It's more like a GetMaximumShiftAmount kind of helper. And there should be an 
update function description that describe a bit more about what is going on. 
I'm not sure why this static function is declared inside ScalarExprEmitter but 
the current description is far away at line 776.

It could say something like this:
Get a maximum legal shift exponent given a shift operation shifting the value 
LHS by RHS steps. The result type is given by RHS. The scalar bit width of the 
LHS value gives an upper bound on the shift exponent as it is considered 
illegal to shift more steps than "width minus one". If that value does not fit 
in the result type, then return an unsinged max for the result type.

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

[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 %s -O0 -fsanitize=shift-exponent -emit-llvm -o - | 
FileCheck %s

bjope wrote:

I think you for example want to use `-std=c2x -triple=x86_64-unknown-linux` 
here.
(Not sure exactly if you for example need the triple, but I fear that you would 
end up with failing build bots otherwise as not all targets support C23 _BitInt 
literals by default.)

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

[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

[Björn Pettersson via cfe-commits]

bjope wrote:

Make sure the commit message refers to fixing #80135.

(Github is a bit weird, so if you want to use the "squash an merge" support in 
the web UI later, then I think you need to update the first comment in the 
"Conversation", rather than updating the commits on this pull-request branch.)

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

[clang] [clang][CGCUDANV] Unify PointerType members of CGNVCUDARuntime (NFC) (PR #75668)

[Björn Pettersson via cfe-commits]

bjope wrote:

I don't know much about this code, so I can't really judge if this is good and 
wanted etc.

When I did opaque pointer cleanups myself earlier (removing some no-op bitcasts 
and using PointerType::get etc) I did leave lots of things like this around on 
purpose, as even if the types are opaque internal to LLVM it might be nice to 
see which kind of types that actually are used and derefenced in the API:s. So 
the differently named variables helps identifying which arguments that points 
to what kind of data.

Anyway, the type cache in CGNVCUDARuntime probably shouldn't store three copies 
of the same unqualified pointer type. One idea, if one still want to keep the 
names, is to use a union like it is done in CodeGenTypeCache.h.

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

[llvm] [clang] [ValueTracking] Add dominating condition support in computeKnownBits() (PR #73662)

[Björn Pettersson via cfe-commits]

bjope wrote:

> > Here is another thing that I noticed after this patch: 
> > https://godbolt.org/z/1P7bnKGjh
> > So early instcombine is eliminating an `and` operation (in the foo 
> > example), resulting in simplifycfg not being able to collapse the control 
> > flow any longer.
> 
> I don't think it's a profitable transformation in simplifycfg with larger 
> amount of instructions. In x86, backend generates much larger code for the 
> example with collapsed cfg: https://godbolt.org/z/YP9GjjP8c.

Optimization pipeline is doing simplifications and canonicalizations. If you 
for example use `-target amdcgn`, then I think you will see that the codegen is 
impacted negatively when not simplifying the control flow. So it depends on the 
backend if one form is profitable or not.
I don't know really which form that should be considered best (simplest and 
easiest for most backends to deal with) here. Just saying that it changed. And 
that could indeed be one reason for regressions (as for our backend).

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

[llvm] [clang] [ValueTracking] Add dominating condition support in computeKnownBits() (PR #73662)

[Björn Pettersson via cfe-commits]

bjope wrote:

Here is another thing that I noticed after this patch: 
https://godbolt.org/z/1P7bnKGjh

So early instcombine is eliminating an `and` operation (in the foo example), 
resulting in simplifycfg not being able to collapse the control flow any longer.

Maybe I should file a separate issue for that?

Not sure if it should be consider as a phase ordering problem. Had perhaps been 
nice if simplifycfg had been able to understand that it basically could match 
an `and` with a no-op by using a -1 mask, etc.

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

[llvm] [clang] [ValueTracking] Add dominating condition support in computeKnownBits() (PR #73662)

[Björn Pettersson via cfe-commits]

bjope wrote:

> @bjope It looks like the InstCombine changes enable IndVars to perform LFTR, 
> which is unprofitable in this case. Though the `umax(1)` call is actually 
> completely unnecessary here, but SCEV doesn't realize it. I've put up #75039 
> to fix that. Does that improve things for you?

@nikic , thanks!
I was thinking that maybe I would need to do something to please the 
SCEVExpander::isHighCostExpansionHelper to prevent the IndVar transform. But 
with #75039 I no longer see the problem umax instructions appearing (and 
increasing the IR instruction count).
I still see some other regressions, but that could be due to limitations in the 
backend. I haven't found anything more that I can blame on this patch.

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

[clang] [llvm] [ValueTracking] Add dominating condition support in computeKnownBits() (PR #73662)

[Björn Pettersson via cfe-commits]

bjope wrote:

Is the behavior seen in this example expected?
   https://godbolt.org/z/f8eqEnsq6

The comments in InstCombineCompares kind of indicates that you try to avoid 
changing a signed predicate to a unsigned. But isn't that what happens here. As 
well as increasing the instruction count by introducing a zext instruction.

Main problem I have with this example is that downstream indvars later is doing
```
INDVARS: Rewriting loop exit condition to:
  LHS:  %inc = add nuw nsw i16 %j.046, 1
   op:  !=
  RHS:%umax = call i16 @llvm.umax.i16(i16 %shr, i16 1)
ExitCount:  (-1 + (1 umax (%n2.053 /u 2)))
  was:   %cmp2 = icmp ult i16 %inc, %shr
```
and that increases the instruction count by inserting an umax in the 
for.body3.preheader. This later seem to result in a cycle regression.

I think you can see something a bit similar if running `opt 
-passes=instcombine,indvars -mtriple i386`.

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

[clang] [llvm] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)

[Björn Pettersson via cfe-commits]

@@ -2282,6 +2282,15 @@ Instruction 
*InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst ) {
   if (MadeChange)
 return 
 
+  // Canonicalize constant GEPs to i8 type.

bjope wrote:

Right. So things can be expected to just work (given getTypeStoreSize(i8)==1), 
even when the addressing unit isn't 8 bits.

Since
```
%gep = getelementptr i3, ptr %p, i16 1
%gep = getelementptr i8, ptr %p, i16 1
%gep = getelementptr i16, ptr %p, i16 1
```
all are equivalent (for my target), then this patch just makes that more 
obvious by canonicalizing them into a single form. 
So we just need to update some lit test checks to expect "getelementptr i8" 
instead of "getelementptr i16" downstream, and hopefully things will be fine.

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

[llvm] [clang] [InstCombine] Canonicalize constant GEPs to i8 source element type (PR #68882)

[Björn Pettersson via cfe-commits]

@@ -2282,6 +2282,15 @@ Instruction 
*InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst ) {
   if (MadeChange)
 return 
 
+  // Canonicalize constant GEPs to i8 type.

bjope wrote:

FWIW, this is a bit interesting downstream with non-8-bit addressing units :-)

Today i8 would be quite rare as GEP type for us as it is smaller than the 
addressing unit size. But I figure that canonicalizing to some other type 
downstream could end up as a lot of work (or lots of missed optimizations).

Afaict accumulateConstantOffset is returning an offset that depends on 
TypeStoreSize. So as long as this is used in a way so that the address still 
would be given by `baseptr +  DL.getTypeStoreSize(i8) * Offset` and not 
`baseptr + 8 * Offset` , then I guess things will be fine (i.e not assuming 
that the offset is an 8-bit offset).

As far as I can tell you could just as well have picked i1 instead of i8 (given 
that `DL.getTypeStoreSize(i1)==DL.getTypeStoreSize(i8)`). That would probably 
look confusing, but that is what happens for us when using i8 as type as we 
can't address individual octets.

(I also see this as a reminder for looking at the ptradd RFC to understand how 
that will impact us, so that we are prepared for that.)

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

[clang] [llvm] [clang-tools-extra] [MCP] Enhance MCP copy Instruction removal for special case (PR #70778)

[Björn Pettersson via cfe-commits]

bjope wrote:

Tracked down some miscompiles found downstream that points at this commit.
Also wrote a new issue here about it: 
https://github.com/llvm/llvm-project/issues/73512

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

[clang-tools-extra] [clang] [llvm] [MachineLICM][AArch64] Hoist COPY instructions with other uses in the loop (PR #71403)

[Björn Pettersson via cfe-commits]

@@ -1262,6 +1262,18 @@ bool MachineLICMBase::IsProfitableToHoist(MachineInstr 
,
 return false;
   }
 
+  // If we have a COPY with other uses in the loop, hoist to allow the users to

bjope wrote:

It is a bit tricky of course, depending on the properties of the target. For a 
VLIW target like ours the COPY could be really cheap, at least if it can be 
executed in parallel with something else without increasing latency. A 
cross-bank copy might actually be fee (zero cycles) even if it is inside the 
loop. OTOH. if we need to spill/reload a register, then that could be much more 
expensive, even if it is hoisted to some outer loop.

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

[llvm] [clang] [clang-tools-extra] [MachineLICM][AArch64] Hoist COPY instructions with other uses in the loop (PR #71403)

[Björn Pettersson via cfe-commits]

@@ -1262,6 +1262,18 @@ bool MachineLICMBase::IsProfitableToHoist(MachineInstr 
,
 return false;
   }
 
+  // If we have a COPY with other uses in the loop, hoist to allow the users to

bjope wrote:

For info: Downstream this caused regressions in some of our benchmarks. Not 
sure if you need to care about that (we will probably guard this with a check 
for our downstream target), but thought it might be nice to mention it.

Haven't fully investigated what happens, but I think that hoisting the COPY 
increases register pressure resulting in spill. The COPY instructions I see 
that are hoisted in that benchmark can be mapped to two categories:
```
%228:gn32 = COPY %143:pn;  cross register bank copy
%245:gn16 = COPY %227.lo:gn32   ; extracting a subreg
```
So for example hoisting the cross register bank COPY results in increasing the 
register pressure for the general gn registers in the path leading up to the 
use. Similarly, by hoisting the subreg extract the register pressure on the gn 
registers increase.

I wonder if the heuristic here perhaps should look closer at the using 
instructions to see if they actually can be hoisted as well? After all, we are 
in the path where CanCauseHighRegPressure has returned true. So traditinonally 
we have been more careful here and only hoisted trivially rematerializable MI:s.

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

[clang-tools-extra] [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (PR #71162)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/71162

From 1fb659fa15568ec8256824339860de724853c6e3 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 2 Nov 2023 22:00:52 +0100
Subject: [PATCH 1/2] [clangd] Check for valid source location in
 ExtractionContext::exprIsValidOutside

If the code has a call to an implicitly declared function, an
expression could end up referencing declarations without valid
source locations. So when doing the exprIsValidOutside check we
could end up calling SourceManager::isPointWithin using invalid
source locations, and then a debug build would crash with an
assertion failure in SourceManager::isBeforeInTranslationUnit.

This patch make sure that we deal with the invalid locations
(by considering a ReferencedDecl with invalid location as not
being inside the Scope).
---
 .../clangd/refactor/tweaks/ExtractVariable.cpp|  3 ++-
 clang-tools-extra/clangd/test/implicit-function.test  | 11 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 clang-tools-extra/clangd/test/implicit-function.test

diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
index 1e4edc6d6b4bb39..79bf962544242bb 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -176,7 +176,8 @@ bool ExtractionContext::exprIsValidOutside(const 
clang::Stmt *Scope) const {
   SourceLocation ScopeBegin = Scope->getBeginLoc();
   SourceLocation ScopeEnd = Scope->getEndLoc();
   for (const Decl *ReferencedDecl : ReferencedDecls) {
-if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) 
&&
+if (ReferencedDecl->getBeginLoc().isValid() &&
+SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) 
&&
 SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd))
   return false;
   }
diff --git a/clang-tools-extra/clangd/test/implicit-function.test 
b/clang-tools-extra/clangd/test/implicit-function.test
new file mode 100644
index 000..85731f358b4ae8e
--- /dev/null
+++ b/clang-tools-extra/clangd/test/implicit-function.test
@@ -0,0 +1,11 @@
+// RUN: cp %s %t.c
+// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck 
-strict-whitespace %s
+
+// Regression test for a situation when we used to hit an assertion failure
+// due to missing source location (for the implicit function declaration).
+
+// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to 
undeclared function 'foo'
+// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is 
not a compile-time constant
+// CHECK: All checks completed, 2 errors
+
+int x = foo();

From 58084d9b753c08be76fe0b171297352e84d61f87 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Fri, 10 Nov 2023 11:54:25 +0100
Subject: [PATCH 2/2] TO BE SQUASHED

Changed the regression test into using unittest.
---
 clang-tools-extra/clangd/test/implicit-function.test  | 11 ---
 .../clangd/unittests/tweaks/ExtractVariableTests.cpp  |  8 
 2 files changed, 8 insertions(+), 11 deletions(-)
 delete mode 100644 clang-tools-extra/clangd/test/implicit-function.test

diff --git a/clang-tools-extra/clangd/test/implicit-function.test 
b/clang-tools-extra/clangd/test/implicit-function.test
deleted file mode 100644
index 85731f358b4ae8e..000
--- a/clang-tools-extra/clangd/test/implicit-function.test
+++ /dev/null
@@ -1,11 +0,0 @@
-// RUN: cp %s %t.c
-// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck 
-strict-whitespace %s
-
-// Regression test for a situation when we used to hit an assertion failure
-// due to missing source location (for the implicit function declaration).
-
-// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to 
undeclared function 'foo'
-// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is 
not a compile-time constant
-// CHECK: All checks completed, 2 errors
-
-int x = foo();
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 3d74a941071f849..eb5b06cfee43166 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -64,6 +64,14 @@ TEST_F(ExtractVariableTest, Test) {
   int x = [[1]];
 })cpp";
   EXPECT_AVAILABLE(AvailableC);
+
+  ExtraArgs = {"-xc"};
+  const char *NoCrashCasesC = R"cpp(
+// error-ok: broken code, but shouldn't crash
+int x = [[foo()]];
+)cpp";
+  EXPECT_UNAVAILABLE(NoCrashCasesC);
+
   ExtraArgs = {"-xobjective-c"};
   const char *AvailableObjC = R"cpp(
 __attribute__((objc_root_class))

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang-tools-extra] [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (PR #71162)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,11 @@
+// RUN: cp %s %t.c

bjope wrote:

Well, I forgot to add the [[]] thing around the call. Now I'm back on track 
again :-)

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

[clang-tools-extra] [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (PR #71162)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,11 @@
+// RUN: cp %s %t.c

bjope wrote:

I've been trying to do that, but so far no luck. I hit this assertion in 
TweakTesting.h:
`  assert(!A.points().empty() || !A.ranges().empty()); 
`

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

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Björn Pettersson via cfe-commits]

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

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Björn Pettersson via cfe-commits]

@@ -0,0 +1,29 @@
+# RUN: clangd -lit-test < %s | FileCheck %s

bjope wrote:

Thanks. I'm not that familiar with the clangd test cases. But I've update the 
pull-request with a unittest instead.

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

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/71415

From 2b1af2bff6432a50eb725adf86e6cfcc52cc020b Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 6 Nov 2023 17:27:28 +0100
Subject: [PATCH 1/2] [clangd] Allow hover over 128-bit variable without
 crashing

When hovering over variables larger than 64 bits, with more than
64 active bits, there were assertion failures since Hover is trying
to print the value as a 64-bit hex value.

There is already protection avoiding to call printHex if there is
more than 64 significant bits. And we already truncate and print
negative values using only 32 bits, when possible. So we can simply
truncate values with more than 64 bits to avoid the assert when using
getZExtValue. The result will be that for example a negative 128 bit
variable is printed using 64 bits, when possible.

There is still no support for printing more than 64 bits. That would
involve more changes since for example llvm::FormatterNumber is
limited to 64 bits.
---
 clang-tools-extra/clangd/Hover.cpp|  4 +++-
 clang-tools-extra/clangd/test/hover2.test | 29 +++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang-tools-extra/clangd/test/hover2.test

diff --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 7f7b5513dff6fee..a868d3bb4e3fa1d 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -408,7 +408,9 @@ void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
 // -2=> 0xfffe
 // -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt ) {
-  uint64_t Bits = V.getZExtValue();
+  assert(V.getSignificantBits() <= 64 && "Can't print more than 64 bits.");
+  uint64_t Bits =
+  V.getBitWidth() > 64 ? V.trunc(64).getZExtValue() : V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);
diff --git a/clang-tools-extra/clangd/test/hover2.test 
b/clang-tools-extra/clangd/test/hover2.test
new file mode 100644
index 000..24d82bde20a7823
--- /dev/null
+++ b/clang-tools-extra/clangd/test/hover2.test
@@ -0,0 +1,29 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"__int128_t
 bar = -4;\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":13}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:   "value": "variable bar\n\nType: __int128_t (aka 
__int128)\nValue = -4 (0xfffc)\n\n__int128_t bar = -4"
+# CHECK-NEXT: },
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:   "end": {
+# CHECK-NEXT: "character": 14,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "start": {
+# CHECK-NEXT: "character": 11,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

From 6e2347e28284e0b7e582fc2d51180b4340f15910 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Tue, 7 Nov 2023 23:38:19 +0100
Subject: [PATCH 2/2] Fixup (should be squashed)

Replace the test case a with a unittest.
---
 clang-tools-extra/clangd/test/hover2.test | 29 ---
 .../clangd/unittests/HoverTests.cpp   | 11 +++
 2 files changed, 11 insertions(+), 29 deletions(-)
 delete mode 100644 clang-tools-extra/clangd/test/hover2.test

diff --git a/clang-tools-extra/clangd/test/hover2.test 
b/clang-tools-extra/clangd/test/hover2.test
deleted file mode 100644
index 24d82bde20a7823..000
--- a/clang-tools-extra/clangd/test/hover2.test
+++ /dev/null
@@ -1,29 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"__int128_t
 bar = -4;\n"}}}

-{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":13}}}
-#  CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": {
-# CHECK-NEXT:"contents": {
-# CHECK-NEXT:  "kind": "plaintext",
-# CHECK-NEXT:   "value": "variable bar\n\nType: __int128_t (aka 
__int128)\nValue = -4 (0xfffc)\n\n__int128_t bar = 

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/71415

When hovering over variables larger than 64 bits, with more than 64 active 
bits, there were assertion failures since Hover is trying to print the value as 
a 64-bit hex value.

There is already protection avoiding to call printHex if there is more than 64 
significant bits. And we already truncate and print negative values using only 
32 bits, when possible. So we can simply truncate values with more than 64 bits 
to avoid the assert when using getZExtValue. The result will be that for 
example a negative 128 bit variable is printed using 64 bits, when possible.

There is still no support for printing more than 64 bits. That would involve 
more changes since for example llvm::FormatterNumber is limited to 64 bits.

From 2b1af2bff6432a50eb725adf86e6cfcc52cc020b Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 6 Nov 2023 17:27:28 +0100
Subject: [PATCH] [clangd] Allow hover over 128-bit variable without crashing

When hovering over variables larger than 64 bits, with more than
64 active bits, there were assertion failures since Hover is trying
to print the value as a 64-bit hex value.

There is already protection avoiding to call printHex if there is
more than 64 significant bits. And we already truncate and print
negative values using only 32 bits, when possible. So we can simply
truncate values with more than 64 bits to avoid the assert when using
getZExtValue. The result will be that for example a negative 128 bit
variable is printed using 64 bits, when possible.

There is still no support for printing more than 64 bits. That would
involve more changes since for example llvm::FormatterNumber is
limited to 64 bits.
---
 clang-tools-extra/clangd/Hover.cpp|  4 +++-
 clang-tools-extra/clangd/test/hover2.test | 29 +++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang-tools-extra/clangd/test/hover2.test

diff --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 7f7b5513dff6fee..a868d3bb4e3fa1d 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -408,7 +408,9 @@ void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
 // -2=> 0xfffe
 // -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt ) {
-  uint64_t Bits = V.getZExtValue();
+  assert(V.getSignificantBits() <= 64 && "Can't print more than 64 bits.");
+  uint64_t Bits =
+  V.getBitWidth() > 64 ? V.trunc(64).getZExtValue() : V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);
diff --git a/clang-tools-extra/clangd/test/hover2.test 
b/clang-tools-extra/clangd/test/hover2.test
new file mode 100644
index 000..24d82bde20a7823
--- /dev/null
+++ b/clang-tools-extra/clangd/test/hover2.test
@@ -0,0 +1,29 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"__int128_t
 bar = -4;\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":13}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:   "value": "variable bar\n\nType: __int128_t (aka 
__int128)\nValue = -4 (0xfffc)\n\n__int128_t bar = -4"
+# CHECK-NEXT: },
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:   "end": {
+# CHECK-NEXT: "character": 14,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "start": {
+# CHECK-NEXT: "character": 11,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (PR #71162)

[Björn Pettersson via cfe-commits]

bjope wrote:

Hi reviewers. I don't know much about clangd so looking for feedback on this 
patch to know if it is the correct thing to do.
Not sure exactly who knows this part of the code, so please let me know if 
someone else should review this!

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

[clang-tools-extra] [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (PR #71162)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/71162

If the code has a call to an implicitly declared function, an expression could 
end up referencing declarations without valid source locations. So when doing 
the exprIsValidOutside check we could end up calling 
SourceManager::isPointWithin using invalid source locations, and then a debug 
build would crash with an assertion failure in 
SourceManager::isBeforeInTranslationUnit.

This patch make sure that we deal with the invalid locations (by considering a 
ReferencedDecl with invalid location as not being inside the Scope).

From 1fb659fa15568ec8256824339860de724853c6e3 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 2 Nov 2023 22:00:52 +0100
Subject: [PATCH] [clangd] Check for valid source location in
 ExtractionContext::exprIsValidOutside

If the code has a call to an implicitly declared function, an
expression could end up referencing declarations without valid
source locations. So when doing the exprIsValidOutside check we
could end up calling SourceManager::isPointWithin using invalid
source locations, and then a debug build would crash with an
assertion failure in SourceManager::isBeforeInTranslationUnit.

This patch make sure that we deal with the invalid locations
(by considering a ReferencedDecl with invalid location as not
being inside the Scope).
---
 .../clangd/refactor/tweaks/ExtractVariable.cpp|  3 ++-
 clang-tools-extra/clangd/test/implicit-function.test  | 11 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 clang-tools-extra/clangd/test/implicit-function.test

diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
index 1e4edc6d6b4bb39..79bf962544242bb 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -176,7 +176,8 @@ bool ExtractionContext::exprIsValidOutside(const 
clang::Stmt *Scope) const {
   SourceLocation ScopeBegin = Scope->getBeginLoc();
   SourceLocation ScopeEnd = Scope->getEndLoc();
   for (const Decl *ReferencedDecl : ReferencedDecls) {
-if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) 
&&
+if (ReferencedDecl->getBeginLoc().isValid() &&
+SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) 
&&
 SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd))
   return false;
   }
diff --git a/clang-tools-extra/clangd/test/implicit-function.test 
b/clang-tools-extra/clangd/test/implicit-function.test
new file mode 100644
index 000..85731f358b4ae8e
--- /dev/null
+++ b/clang-tools-extra/clangd/test/implicit-function.test
@@ -0,0 +1,11 @@
+// RUN: cp %s %t.c
+// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck 
-strict-whitespace %s
+
+// Regression test for a situation when we used to hit an assertion failure
+// due to missing source location (for the implicit function declaration).
+
+// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to 
undeclared function 'foo'
+// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is 
not a compile-time constant
+// CHECK: All checks completed, 2 errors
+
+int x = foo();

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Björn Pettersson via cfe-commits]

@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

bjope wrote:

Oops, sorry. Now I think my previous comment was wrong. I missed the 
`getActiveBits()` check in APInt::ult. So it should be fine without 
complicating it that way.

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

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Björn Pettersson via cfe-commits]

@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

bjope wrote:

I tried to understand why the old code used an APInt here.
What if you have something like ` x >> larger_than_64_bits_value`? I suspect 
that then the uge helper here will do a 64-bit unsigned compare. So it will 
truncate `Right` to 64 bits instead, right?

So I think the easiest way would be to compare to APInt:s. But make sure those 
are zero-extended to have a common size.
Maybe something like this:
```
  unsigned CompareBits = std::max(Right.getBitWidth(), 64);
  llvm::APInt LeftBits(CompareBits, LeftSize);
  if (Right.zext(CompareBits).uge(LeftBits)) {
```

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

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

bjope wrote:

> > I guess I don't know how pull requests and reviewing works in github. I 
> > actually added 3 comments on this patch a several days (or weeks) ago. But 
> > turns out that they were "pending" because I had only "started review" and 
> > not found the place to "submit review".
> 
> For that reason I usually click "add single comment" instead of "start a 
> review".

Thanks. I'll try to remember that.

If they for example had named the buttons "Post comment directly" and "Save as 
draft", then it has been more obvious.
I also do not know how to find all my pending "draft reviews" (unless I'm 
already "assigned" or "requested as reviewer") as the https://github.com/pulls 
is just crap compared to my old dashboard in phabricator.

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

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

@@ -3280,7 +3280,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
 if (skipFunction(F))
   return false;
 
-auto *LIWP = getAnalysisIfAvailable();
+auto  = getAnalysis();

bjope wrote:

IIUC GVN doesn't depend on LoopInfo itself. It should just make sure to 
preserve it if it is available. So one might wonder if it is the addRequire 
that should be questioned and possibly removed instead.
But it's been like this for years (that it is required), and messing around 
with the legacy PM analysis dependencies for an IR pass (even if it is used by 
some backends) is perhaps not worth it.

Therefore I think you proposed change is ok. Or you could just leave this as 
is, since the IfAvailable part in some sense indicate that the pass itself 
doesn't depend on the analysis.

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

[clang] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

@@ -935,7 +935,7 @@ bool 
AArch64ConditionalCompares::runOnMachineFunction(MachineFunction ) {
   SchedModel = MF.getSubtarget().getSchedModel();
   MRI = ();
   DomTree = ();
-  Loops = getAnalysisIfAvailable();
+  Loops = ();

bjope wrote:

I think there are some "weird" things going on here. But maybe not worth 
digging into those unless being an AArch64 maintainer.

Afaict the goal here is to make sure MachineLoopInfo is preserved. So this pass 
shouldn't really need to require it explicitly. However, the 
MachineTraceMetrics pass is also using MachineLoopInfo transitively. So, the 
MachineLoopInfo analysis will be run. Although I think there is a "bug" in 
MachineTraceMetrics related to not using addRequiredTransitive. So things might 
break if removing the addRequired in this pass.

To sum up. The proposed change is probably harmless and OK. Just saying that 
the analysis pass dependencies used in here seem to be a bit ad-hoc(?).

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

[clang] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

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

[clang] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

@@ -261,7 +261,7 @@ bool VirtRegRewriter::runOnMachineFunction(MachineFunction 
) {
   Indexes = ();
   LIS = ();
   VRM = ();
-  DebugVars = getAnalysisIfAvailable();
+  DebugVars = ();

bjope wrote:

Not sure how to add a comment on line 278 if I want to suggest an edit on that 
line (in Phabricator it would have been easy). But I guess the check if 
DebugVars is null or not on line 278 is redundant now.

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

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

https://github.com/bjope commented:

I guess I don't know how pull requests and reviewing works in github. I 
actually added 3 comments on this patch a several days (or weeks) ago. But 
turns out that they were "pending" because I had only "started review" and not 
found the place to "submit review".

So sorry for some late comments here. Since this has been pushed already, then 
I guess you may ignore them. 

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

[clang] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

@@ -3280,7 +3280,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
 if (skipFunction(F))
   return false;
 
-auto *LIWP = getAnalysisIfAvailable();
+auto  = getAnalysis();

bjope wrote:

IIUC GVN doesn't depend on LoopInfo itself. It should just make sure to 
preserve it if it is available. So one might wonder if it is the addRequire 
that should be questioned and possibly removed instead.
But it's been like this for years (that it is required), and messing around 
with the legacy PM analysis dependencies for an IR pass (even if it is used by 
some backends) is perhaps not worth it.

Therefore I think you proposed change is ok. Or you could just leave this as 
is, since the IfAvailable part in some sense indicate that the pass itself 
doesn't depend on the analysis.

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

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

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

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

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

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/68590

From 8ef4560759280bbc14e0dc6c01efc036d0410dca Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 16:13:39 +0200
Subject: [PATCH] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/docs/ReleaseNotes.rst  |  4 
 clang/lib/Sema/SemaChecking.cpp  |  5 ++---
 clang/test/Sema/c2x-expr-range.c | 14 ++
 3 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Sema/c2x-expr-range.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c320bc8b35d23d..2d918967e7f0b02 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -378,6 +378,10 @@ Bug Fixes in This Version
 - Fix crash in evaluating ``constexpr`` value for invalid template function.
   Fixes (`#68542 `_)
 
+- Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right
+  shift operation, could result in missing warnings about
+  ``shift count >= width of type`` or internal compiler error.
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 446e35218bff0ad..2594a8f97f7d94e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13596,11 +13596,10 @@ static IntRange GetExprRange(ASTContext , const 
Expr *E, unsigned MaxWidth,
   if (std::optional shift =
   BO->getRHS()->getIntegerConstantExpr(C)) {
 if (shift->isNonNegative()) {
-  unsigned zext = shift->getZExtValue();
-  if (zext >= L.Width)
+  if (shift->uge(L.Width))
 L.Width = (L.NonNegative ? 0 : 1);
   else
-L.Width -= zext;
+L.Width -= shift->getZExtValue();
 }
   }
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
new file mode 100644
index 000..73683e6bfe684aa
--- /dev/null
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
+
+// Regression test for bug where we used to hit an assertion due to shift 
amount
+// being larger than 64 bits. We want to see a warning about too large shift
+// amount.
+void test1(int *a) {
+  (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift 
count >= width of type}}
+}
+
+// Similar to test1 above, but using __uint128_t instead of __BitInt.
+// We want to see a warning about too large shift amount.
+void test2(__uint128_t *a) {
+  (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/68590

From 44eedec9dae39e72d5474426a15a07d9be4144fc Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 16:13:39 +0200
Subject: [PATCH 1/3] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/lib/Sema/SemaChecking.cpp  |  5 ++---
 clang/test/Sema/c2x-expr-range.c | 20 
 2 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Sema/c2x-expr-range.c

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3932d9cd07d9864..eb4d8d2e2806bcb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13554,11 +13554,10 @@ static IntRange GetExprRange(ASTContext , const 
Expr *E, unsigned MaxWidth,
   if (std::optional shift =
   BO->getRHS()->getIntegerConstantExpr(C)) {
 if (shift->isNonNegative()) {
-  unsigned zext = shift->getZExtValue();
-  if (zext >= L.Width)
+  if (shift->uge(L.Width))
 L.Width = (L.NonNegative ? 0 : 1);
   else
-L.Width -= zext;
+L.Width -= shift->getZExtValue();
 }
   }
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
new file mode 100644
index 000..1690a6280386bd5
--- /dev/null
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
+
+// test1 used to hit an assertion failure like this during parsing:
+// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t 
llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too 
many bits for uint64_t"' failed.
+// With a stack trace like this leading up to the assert:
+//
+//  #9 0x5645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, 
unsigned int, bool, bool) SemaChecking.cpp:0:0
+// #10 0x5645bca048a8 AnalyzeImplicitConversions(clang::Sema&, 
clang::Expr*, clang::SourceLocation, bool) SemaChecking.cpp:0:0
+// #11 0x5645bca06af1 clang::Sema::CheckCompletedExpr(clang::Expr*, 
clang::SourceLocation, bool)
+//
+void test1(int *a)
+{
+  (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift 
count >= width of type}}
+}
+
+// Same as above but using __uint128_t instead of __BitInt.
+void test2(__uint128_t *a)
+{
+  (void)(*a >> ((__uint128_t)__UINT64_MAX__ + (__uint128_t)1) <= 0); // 
expected-warning {{shift count >= width of type}}
+}

From f8881531c6b5f8a7f71554c01c4e7df8e24e7f72 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 17:34:41 +0200
Subject: [PATCH 2/3] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/docs/ReleaseNotes.rst  |  4 
 clang/test/Sema/c2x-expr-range.c | 14 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2d0302c399fb6f3..45ee6db4112b978 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -219,6 +219,10 @@ Bug Fixes in This Version
 - Clang no longer considers the loss of ``__unaligned`` qualifier from objects 
as
   an invalid conversion during method function overload resolution.
 
+- Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right
+  shift operation, could result in missing warnings about
+  ``shift count >= width of type`` or internal compiler error.
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 1690a6280386bd5..0fe1bc547bbd353 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -1,19 +1,15 @@
 // RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
 
-// test1 used to hit an assertion failure like this during parsing:
-// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t 
llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too 
many bits for uint64_t"' failed.
-// With a stack trace like this leading up to the assert:
-//
-//  #9 0x5645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, 
unsigned int, bool, bool) 

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

@@ -219,6 +219,10 @@ Bug Fixes in This Version
 - Clang no longer considers the loss of ``__unaligned`` qualifier from objects 
as
   an invalid conversion during method function overload resolution.
 
+- Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right

bjope wrote:

> Is there an issue filed in the issue tracker for this bug?

Not that I'm aware of. Problem was found in downstream fuzzy testing involving 
BitInt types.

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

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

bjope wrote:

> So the change itself looks right. The comment at the top of the test belongs 
> in the commit message/github discussion, not in the test itself.
> 
> Also, needs a release note.

Thanks. I pushed an update. It fixes the test case to just explain the purpose 
of the tests. And I added info to the ReleaseNotes.

Note: I've never edited release notes for clang in the past, so please let me 
know if that should go somewhere else or if I missed something about what it 
should look like, etc. 

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

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/68590

From 44eedec9dae39e72d5474426a15a07d9be4144fc Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 16:13:39 +0200
Subject: [PATCH 1/2] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/lib/Sema/SemaChecking.cpp  |  5 ++---
 clang/test/Sema/c2x-expr-range.c | 20 
 2 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Sema/c2x-expr-range.c

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3932d9cd07d9864..eb4d8d2e2806bcb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13554,11 +13554,10 @@ static IntRange GetExprRange(ASTContext , const 
Expr *E, unsigned MaxWidth,
   if (std::optional shift =
   BO->getRHS()->getIntegerConstantExpr(C)) {
 if (shift->isNonNegative()) {
-  unsigned zext = shift->getZExtValue();
-  if (zext >= L.Width)
+  if (shift->uge(L.Width))
 L.Width = (L.NonNegative ? 0 : 1);
   else
-L.Width -= zext;
+L.Width -= shift->getZExtValue();
 }
   }
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
new file mode 100644
index 000..1690a6280386bd5
--- /dev/null
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
+
+// test1 used to hit an assertion failure like this during parsing:
+// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t 
llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too 
many bits for uint64_t"' failed.
+// With a stack trace like this leading up to the assert:
+//
+//  #9 0x5645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, 
unsigned int, bool, bool) SemaChecking.cpp:0:0
+// #10 0x5645bca048a8 AnalyzeImplicitConversions(clang::Sema&, 
clang::Expr*, clang::SourceLocation, bool) SemaChecking.cpp:0:0
+// #11 0x5645bca06af1 clang::Sema::CheckCompletedExpr(clang::Expr*, 
clang::SourceLocation, bool)
+//
+void test1(int *a)
+{
+  (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift 
count >= width of type}}
+}
+
+// Same as above but using __uint128_t instead of __BitInt.
+void test2(__uint128_t *a)
+{
+  (void)(*a >> ((__uint128_t)__UINT64_MAX__ + (__uint128_t)1) <= 0); // 
expected-warning {{shift count >= width of type}}
+}

From f8881531c6b5f8a7f71554c01c4e7df8e24e7f72 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 17:34:41 +0200
Subject: [PATCH 2/2] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/docs/ReleaseNotes.rst  |  4 
 clang/test/Sema/c2x-expr-range.c | 14 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2d0302c399fb6f3..45ee6db4112b978 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -219,6 +219,10 @@ Bug Fixes in This Version
 - Clang no longer considers the loss of ``__unaligned`` qualifier from objects 
as
   an invalid conversion during method function overload resolution.
 
+- Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right
+  shift operation, could result in missing warnings about
+  ``shift count >= width of type`` or internal compiler error.
+
 Bug Fixes to Compiler Builtins
 ^^
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 1690a6280386bd5..0fe1bc547bbd353 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -1,19 +1,15 @@
 // RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
 
-// test1 used to hit an assertion failure like this during parsing:
-// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t 
llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too 
many bits for uint64_t"' failed.
-// With a stack trace like this leading up to the assert:
-//
-//  #9 0x5645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, 
unsigned int, bool, bool) 

[clang] [Sema] Handle large shift counts in GetExprRange (PR #68590)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/68590

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.


From 44eedec9dae39e72d5474426a15a07d9be4144fc Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Mon, 9 Oct 2023 16:13:39 +0200
Subject: [PATCH] [Sema] Handle large shift counts in GetExprRange

GetExprRange did not expect that very large shift counts when
narrowing the range based on logical right shifts. So with inputs
such as
*a >> 123456789012345678901uwb
it would hit assertions about trying to convert a too large APInt
into uint64_t.
This patch fixes that by using the APInt value when determining if
we should reduce the range by the shift count or not.
---
 clang/lib/Sema/SemaChecking.cpp  |  5 ++---
 clang/test/Sema/c2x-expr-range.c | 20 
 2 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Sema/c2x-expr-range.c

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3932d9cd07d9864..eb4d8d2e2806bcb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13554,11 +13554,10 @@ static IntRange GetExprRange(ASTContext , const 
Expr *E, unsigned MaxWidth,
   if (std::optional shift =
   BO->getRHS()->getIntegerConstantExpr(C)) {
 if (shift->isNonNegative()) {
-  unsigned zext = shift->getZExtValue();
-  if (zext >= L.Width)
+  if (shift->uge(L.Width))
 L.Width = (L.NonNegative ? 0 : 1);
   else
-L.Width -= zext;
+L.Width -= shift->getZExtValue();
 }
   }
 
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
new file mode 100644
index 000..1690a6280386bd5
--- /dev/null
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux 
%s
+
+// test1 used to hit an assertion failure like this during parsing:
+// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t 
llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too 
many bits for uint64_t"' failed.
+// With a stack trace like this leading up to the assert:
+//
+//  #9 0x5645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, 
unsigned int, bool, bool) SemaChecking.cpp:0:0
+// #10 0x5645bca048a8 AnalyzeImplicitConversions(clang::Sema&, 
clang::Expr*, clang::SourceLocation, bool) SemaChecking.cpp:0:0
+// #11 0x5645bca06af1 clang::Sema::CheckCompletedExpr(clang::Expr*, 
clang::SourceLocation, bool)
+//
+void test1(int *a)
+{
+  (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift 
count >= width of type}}
+}
+
+// Same as above but using __uint128_t instead of __BitInt.
+void test2(__uint128_t *a)
+{
+  (void)(*a >> ((__uint128_t)__UINT64_MAX__ + (__uint128_t)1) <= 0); // 
expected-warning {{shift count >= width of type}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Replace uses of CreatePointerBitCastOrAddrSpaceCast (NFC) (PR #68277)

[Björn Pettersson via cfe-commits]

@@ -1123,9 +1123,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
 if (!CurFnInfo->getReturnInfo().getIndirectByVal()) {
   ReturnValuePointer =
   CreateDefaultAlignTempAlloca(Int8PtrTy, "result.ptr");
-  Builder.CreateStore(Builder.CreatePointerBitCastOrAddrSpaceCast(
-  ReturnValue.getPointer(), Int8PtrTy),
-  ReturnValuePointer);
+  Builder.CreateStore(ReturnValue.getPointer(), ReturnValuePointer);

bjope wrote:

So `ReturnValue.getPointer()` is guaranteed to be an unqualified pointer?

I'm just wondering why this hasn't been just a pointer bitcast in the past. But 
even if it would cast away an address space it seems a bit weird.
Not sure if it is worth an assert now when we bypass all the castIsValid checks 
etc that would have been done in the past.

(And maybe the "result.ptr" should be created with 
`ReturnValue.getPointer().getType()` rather than using Int8PtrTy. Although I 
consider that as out of scope for this patch.)

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

[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][driver] Use platform specific calls to get the executable absolute path (PR #68091)

[Björn Pettersson via cfe-commits]

@@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl 
,
   // path being a symlink.
   SmallString<128> InstalledPath(argv[0]);
 
+#if defined(__linux__)

bjope wrote:

I was kind of thinking the same. And then started to wonder about why 
SetInstallDir says "We do this manually, because we want to support that path 
being a symlink.". I don't really understand if that indicates that the 
installed dir should be set based on the symlinks path or not.

Afaict `Driver::ClangExecutable` already is computed via `getMainExecutable()`, 
right. 
And the Driver c'tor is setting both `Driver::Dir` and `Driver::InstalledDir` 
based on the path of the clang executable.

So is the problem here is that SetInstallDir is called after the above (having 
contructed `TheDriver`). Resulting in `InstalledDir` being changed into being 
relative to the symlink?

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/65624

From caa626ea4e813c7061b44f6b4336f31fce511f4b Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 7 Sep 2023 13:08:22 +0200
Subject: [PATCH] [clang][CodeGen] Simplify code based on opaque pointers

- Update CodeGenTypeCache to use a single union for all pointers in
  address space zero.
- Introduce a UnqualPtrTy in CodeGenTypeCache, and use that (for
  example instead of llvm::PointerType::getUnqual) in some places.
- Drop some redundant bit/pointers casts from ptr to ptr.
---
 clang/lib/CodeGen/CGAtomic.cpp|  3 +-
 clang/lib/CodeGen/CGBlocks.cpp| 20 ++
 clang/lib/CodeGen/CGBuiltin.cpp   | 99 ---
 clang/lib/CodeGen/CGCUDANV.cpp|  9 +--
 clang/lib/CodeGen/CGExpr.cpp  |  4 +-
 clang/lib/CodeGen/CGObjCRuntime.cpp   |  4 +-
 clang/lib/CodeGen/CGOpenCLRuntime.cpp | 16 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   | 13 ++--
 clang/lib/CodeGen/CodeGenTypeCache.h  |  7 +-
 clang/lib/CodeGen/ItaniumCXXABI.cpp   | 39 +--
 clang/lib/CodeGen/Targets/PPC.cpp |  2 +-
 clang/lib/CodeGen/Targets/Sparc.cpp   |  2 +-
 clang/lib/CodeGen/Targets/X86.cpp | 10 +--
 13 files changed, 81 insertions(+), 147 deletions(-)

diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 222b0a192c85e20..83ad6739015b8d2 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -87,8 +87,7 @@ namespace {
 llvm::Value *StoragePtr = CGF.Builder.CreateConstGEP1_64(
 CGF.Int8Ty, BitFieldPtr, OffsetInChars.getQuantity());
 StoragePtr = CGF.Builder.CreateAddrSpaceCast(
-StoragePtr, llvm::PointerType::getUnqual(CGF.getLLVMContext()),
-"atomic_bitfield_base");
+StoragePtr, CGF.UnqualPtrTy, "atomic_bitfield_base");
 BFI = OrigBFI;
 BFI.Offset = Offset;
 BFI.StorageSize = AtomicSizeInBits;
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 4f64012fc1a5c39..aa3e730d28efae5 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1189,8 +1189,8 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
 }
   } else {
 // Bitcast the block literal to a generic block literal.
-BlockPtr = Builder.CreatePointerCast(
-BlockPtr, llvm::PointerType::get(GenBlockTy, 0), "block.literal");
+BlockPtr =
+Builder.CreatePointerCast(BlockPtr, UnqualPtrTy, "block.literal");
 // Get pointer to the block invoke function
 llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 3);
 
@@ -1208,12 +1208,6 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
   const CGFunctionInfo  =
 CGM.getTypes().arrangeBlockFunctionCall(Args, FuncTy);
 
-  // Cast the function pointer to the right type.
-  llvm::Type *BlockFTy = CGM.getTypes().GetFunctionType(FnInfo);
-
-  llvm::Type *BlockFTyPtr = llvm::PointerType::getUnqual(BlockFTy);
-  Func = Builder.CreatePointerCast(Func, BlockFTyPtr);
-
   // Prepare the callee.
   CGCallee Callee(CGCalleeInfo(), Func);
 
@@ -2589,11 +2583,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   SmallVector types;
 
   // void *__isa;
-  types.push_back(Int8PtrTy);
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // void *__forwarding;
-  types.push_back(llvm::PointerType::getUnqual(byrefType));
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // int32_t __flags;
@@ -2608,11 +2602,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   bool hasCopyAndDispose = getContext().BlockRequiresCopying(Ty, D);
   if (hasCopyAndDispose) {
 /// void *__copy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
 
 /// void *__destroy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
   }
 
@@ -2621,7 +2615,7 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   if (getContext().getByrefLifetime(Ty, Lifetime, HasByrefExtendedLayout) &&
   HasByrefExtendedLayout) {
 /// void *__byref_variable_layout;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += CharUnits::fromQuantity(PointerSizeInBytes);
   }
 
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index c175f274319b8c4..d14cf0dccb09982 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -978,9 +978,8 @@ static llvm::Value *EmitX86BitTestIntrinsic(CodeGenFunction 
,
   llvm::IntegerType *IntType = llvm::IntegerType::get(
   CGF.getLLVMContext(),
   CGF.getContext().getTypeSize(E->getArg(1)->getType()));
-  llvm::Type *PtrType = llvm::PointerType::getUnqual(CGF.getLLVMContext());
   llvm::FunctionType *FTy =
-  llvm::FunctionType::get(CGF.Int8Ty, {PtrType, IntType}, false);
+ 

[clang] [analyzer] Fix crash analyzing _BitInt() in evalIntegralCast (PR #65887)

[Björn Pettersson via cfe-commits]

bjope wrote:

I reverted this patch since buildbots have been complaining for almost an hour.
Here is the first failure mail I got:

```
The Buildbot has detected a failed build on builder clang-armv8-quick while 
building clang.

Full details are available at:
https://lab.llvm.org/buildbot/#/builders/245/builds/14207

Worker for this Build: linaro-clang-armv8-quick

BUILD FAILED: failed 1 unexpected failures 38187 expected passes 71 expected 
failures 36091 unsupported tests (failure)

Step 5 (ninja check 1) failure: 1 unexpected failures 38187 expected passes 71 
expected failures 36091 unsupported tests (failure)
 TEST 'Clang :: Analysis/bitint-no-crash.c' FAILED 

Script:
--
: 'RUN: at line 1';   
/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang -cc1 
-internal-isystem 
/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/lib/clang/18/include 
-nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer 
-analyzer-checker=core-analyzer-checker=debug.ExprInspection-verify 
/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/bitint-no-crash.c
--
Exit Code: 1

Command Output (stderr):
--
error: 'expected-error' diagnostics seen but not expected: 
  File 
/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/bitint-no-crash.c
 Line 7: signed _BitInt of bit sizes greater than 128 not supported
  File 
/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/bitint-no-crash.c
 Line 8: signed _BitInt of bit sizes greater than 128 not supported
2 errors generated.

--
```

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

[clang] Update GoogleTest to v1.14.0 (PR #65823)

[Björn Pettersson via cfe-commits]

bjope wrote:

> > Ok, seems like the only problem is the MemProf unit test 
> > compiler-rt/lib/memprof/tests/
> > So if I manually disable that I don't see any other failures with this 
> > patch.
> 
> I think any components that uses freshly built clang (stage1) could 
> potentially run into this problem again, the correct way to solve it is to 
> provide a compatible C++ Library to the freshly built clang when it built 
> compiler-rt and related tests. For example, use -D with 
> `RUNTIMES_${target}_CMAKE_CXX_FLAGS` and `RUNTIMES_${target}_CMAKE_CXX_FLAGS` 
> to pass `--gcc-toolchain` (it might be deprecated and there is an updated 
> flag for it) to the runtimes build. This will probably allow stage1 clang to 
> use gcc's stdlib like your bootstrap clang.

Thanks! Yes, I think we hadn't considered that some things we set up for the 
regular build wasn't forwarded to the compiler-rt test.
When setting
  
`-DRUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS=--gcc-toolchain=`
the problem @mikaelholmen mentioned disappeared. I haven't tried adding it to 
CMAKE_CXX_FLAGS, but maybe that is a better option.

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

@@ -51,14 +51,11 @@ struct CodeGenTypeCache {
 llvm::IntegerType *PtrDiffTy;
   };
 
-  /// void* in address space 0
+  /// void*, void** in address space 0
   union {
+llvm::PointerType *UnqualPtrTy;

bjope wrote:

Ok , thanks. Then I assume the name is fine.

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

https://github.com/bjope updated 
https://github.com/llvm/llvm-project/pull/65624:

From 6ccf70a6aa245b83667ef6547c869035dd06da6f Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 7 Sep 2023 13:08:22 +0200
Subject: [PATCH] [clang][CodeGen] Simplify code based on opaque pointers

- Update CodeGenTypeCache to use a single union for all pointers in
  address space zero.
- Introduce a UnqualPtrTy in CodeGenTypeCache, and use that (for
  example instead of llvm::PointerType::getUnqual) in some places.
- Drop some redundant bit/pointers casts from ptr to ptr.
---
 clang/lib/CodeGen/CGAtomic.cpp|  3 +-
 clang/lib/CodeGen/CGBlocks.cpp| 20 ++
 clang/lib/CodeGen/CGBuiltin.cpp   | 99 ---
 clang/lib/CodeGen/CGCUDANV.cpp|  9 +--
 clang/lib/CodeGen/CGExpr.cpp  |  4 +-
 clang/lib/CodeGen/CGObjCRuntime.cpp   |  4 +-
 clang/lib/CodeGen/CGOpenCLRuntime.cpp | 16 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   | 13 ++--
 clang/lib/CodeGen/CodeGenTypeCache.h  |  7 +-
 clang/lib/CodeGen/ItaniumCXXABI.cpp   | 39 +--
 clang/lib/CodeGen/Targets/PPC.cpp |  2 +-
 clang/lib/CodeGen/Targets/Sparc.cpp   |  2 +-
 clang/lib/CodeGen/Targets/X86.cpp | 10 +--
 13 files changed, 81 insertions(+), 147 deletions(-)

diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 222b0a192c85e20..83ad6739015b8d2 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -87,8 +87,7 @@ namespace {
 llvm::Value *StoragePtr = CGF.Builder.CreateConstGEP1_64(
 CGF.Int8Ty, BitFieldPtr, OffsetInChars.getQuantity());
 StoragePtr = CGF.Builder.CreateAddrSpaceCast(
-StoragePtr, llvm::PointerType::getUnqual(CGF.getLLVMContext()),
-"atomic_bitfield_base");
+StoragePtr, CGF.UnqualPtrTy, "atomic_bitfield_base");
 BFI = OrigBFI;
 BFI.Offset = Offset;
 BFI.StorageSize = AtomicSizeInBits;
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 4f64012fc1a5c39..aa3e730d28efae5 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1189,8 +1189,8 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
 }
   } else {
 // Bitcast the block literal to a generic block literal.
-BlockPtr = Builder.CreatePointerCast(
-BlockPtr, llvm::PointerType::get(GenBlockTy, 0), "block.literal");
+BlockPtr =
+Builder.CreatePointerCast(BlockPtr, UnqualPtrTy, "block.literal");
 // Get pointer to the block invoke function
 llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 3);
 
@@ -1208,12 +1208,6 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
   const CGFunctionInfo  =
 CGM.getTypes().arrangeBlockFunctionCall(Args, FuncTy);
 
-  // Cast the function pointer to the right type.
-  llvm::Type *BlockFTy = CGM.getTypes().GetFunctionType(FnInfo);
-
-  llvm::Type *BlockFTyPtr = llvm::PointerType::getUnqual(BlockFTy);
-  Func = Builder.CreatePointerCast(Func, BlockFTyPtr);
-
   // Prepare the callee.
   CGCallee Callee(CGCalleeInfo(), Func);
 
@@ -2589,11 +2583,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   SmallVector types;
 
   // void *__isa;
-  types.push_back(Int8PtrTy);
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // void *__forwarding;
-  types.push_back(llvm::PointerType::getUnqual(byrefType));
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // int32_t __flags;
@@ -2608,11 +2602,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   bool hasCopyAndDispose = getContext().BlockRequiresCopying(Ty, D);
   if (hasCopyAndDispose) {
 /// void *__copy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
 
 /// void *__destroy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
   }
 
@@ -2621,7 +2615,7 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   if (getContext().getByrefLifetime(Ty, Lifetime, HasByrefExtendedLayout) &&
   HasByrefExtendedLayout) {
 /// void *__byref_variable_layout;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += CharUnits::fromQuantity(PointerSizeInBytes);
   }
 
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 52868ca260290b7..18f2a03c7995233 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -978,9 +978,8 @@ static llvm::Value *EmitX86BitTestIntrinsic(CodeGenFunction 
,
   llvm::IntegerType *IntType = llvm::IntegerType::get(
   CGF.getLLVMContext(),
   CGF.getContext().getTypeSize(E->getArg(1)->getType()));
-  llvm::Type *PtrType = llvm::PointerType::getUnqual(CGF.getLLVMContext());
   llvm::FunctionType *FTy =
-  llvm::FunctionType::get(CGF.Int8Ty, {PtrType, IntType}, false);
+   

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

@@ -51,14 +51,11 @@ struct CodeGenTypeCache {
 llvm::IntegerType *PtrDiffTy;
   };
 
-  /// void* in address space 0
+  /// void*, void** in address space 0
   union {
+llvm::PointerType *UnqualPtrTy;

bjope wrote:

Maybe `OpaquePtrTy` is a better name. But `Unqual` seem to be used in other 
places and I've typically used this instead of `PointerType::getUnqual`. But 
I'm not sure what `unqual` has been denoting originally (a pointer in address 
space zero?).

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

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

[clang] [clang][CodeGen] Simplify code based on opaque pointers (PR #65624)

[Björn Pettersson via cfe-commits]

https://github.com/bjope created 
https://github.com/llvm/llvm-project/pull/65624:

- Update CodeGenTypeCache to use a single union for all pointers in
  address space zero.
- Introduce a UnqualPtrTy in CodeGenTypeCache, and use that (for
  example instead of llvm::PointerType::getUnqual) in some places.
- Drop some redundant bit/pointers casts from ptr to ptr.


From 5431b8469b3ebdc92e5dadc9889b7b133da1c38d Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson 
Date: Thu, 7 Sep 2023 13:08:22 +0200
Subject: [PATCH] [clang][CodeGen] Simplify code based on opaque pointers

- Update CodeGenTypeCache to use a single union for all pointers in
  address space zero.
- Introduce a UnqualPtrTy in CodeGenTypeCache, and use that (for
  example instead of llvm::PointerType::getUnqual) in some places.
- Drop some redundant bit/pointers casts from ptr to ptr.
---
 clang/lib/CodeGen/CGAtomic.cpp|  3 +-
 clang/lib/CodeGen/CGBlocks.cpp| 20 ++
 clang/lib/CodeGen/CGBuiltin.cpp   | 99 ---
 clang/lib/CodeGen/CGCUDANV.cpp|  9 +--
 clang/lib/CodeGen/CGExpr.cpp  |  4 +-
 clang/lib/CodeGen/CGObjCRuntime.cpp   |  4 +-
 clang/lib/CodeGen/CGOpenCLRuntime.cpp | 16 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   | 13 ++--
 clang/lib/CodeGen/CodeGenTypeCache.h  |  7 +-
 clang/lib/CodeGen/ItaniumCXXABI.cpp   | 39 +--
 clang/lib/CodeGen/Targets/PPC.cpp |  2 +-
 clang/lib/CodeGen/Targets/Sparc.cpp   |  2 +-
 clang/lib/CodeGen/Targets/X86.cpp | 10 +--
 13 files changed, 81 insertions(+), 147 deletions(-)

diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 222b0a192c85e20..83ad6739015b8d2 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -87,8 +87,7 @@ namespace {
 llvm::Value *StoragePtr = CGF.Builder.CreateConstGEP1_64(
 CGF.Int8Ty, BitFieldPtr, OffsetInChars.getQuantity());
 StoragePtr = CGF.Builder.CreateAddrSpaceCast(
-StoragePtr, llvm::PointerType::getUnqual(CGF.getLLVMContext()),
-"atomic_bitfield_base");
+StoragePtr, CGF.UnqualPtrTy, "atomic_bitfield_base");
 BFI = OrigBFI;
 BFI.Offset = Offset;
 BFI.StorageSize = AtomicSizeInBits;
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 4f64012fc1a5c39..aa3e730d28efae5 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1189,8 +1189,8 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
 }
   } else {
 // Bitcast the block literal to a generic block literal.
-BlockPtr = Builder.CreatePointerCast(
-BlockPtr, llvm::PointerType::get(GenBlockTy, 0), "block.literal");
+BlockPtr =
+Builder.CreatePointerCast(BlockPtr, UnqualPtrTy, "block.literal");
 // Get pointer to the block invoke function
 llvm::Value *FuncPtr = Builder.CreateStructGEP(GenBlockTy, BlockPtr, 3);
 
@@ -1208,12 +1208,6 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr 
*E,
   const CGFunctionInfo  =
 CGM.getTypes().arrangeBlockFunctionCall(Args, FuncTy);
 
-  // Cast the function pointer to the right type.
-  llvm::Type *BlockFTy = CGM.getTypes().GetFunctionType(FnInfo);
-
-  llvm::Type *BlockFTyPtr = llvm::PointerType::getUnqual(BlockFTy);
-  Func = Builder.CreatePointerCast(Func, BlockFTyPtr);
-
   // Prepare the callee.
   CGCallee Callee(CGCalleeInfo(), Func);
 
@@ -2589,11 +2583,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   SmallVector types;
 
   // void *__isa;
-  types.push_back(Int8PtrTy);
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // void *__forwarding;
-  types.push_back(llvm::PointerType::getUnqual(byrefType));
+  types.push_back(VoidPtrTy);
   size += getPointerSize();
 
   // int32_t __flags;
@@ -2608,11 +2602,11 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   bool hasCopyAndDispose = getContext().BlockRequiresCopying(Ty, D);
   if (hasCopyAndDispose) {
 /// void *__copy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
 
 /// void *__destroy_helper;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += getPointerSize();
   }
 
@@ -2621,7 +2615,7 @@ const BlockByrefInfo 
::getBlockByrefInfo(const VarDecl *D) {
   if (getContext().getByrefLifetime(Ty, Lifetime, HasByrefExtendedLayout) &&
   HasByrefExtendedLayout) {
 /// void *__byref_variable_layout;
-types.push_back(Int8PtrTy);
+types.push_back(VoidPtrTy);
 size += CharUnits::fromQuantity(PointerSizeInBytes);
   }
 
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a513eae46e358e5..cbd425a1cf2a109 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -978,9 +978,8 @@ static llvm::Value *EmitX86BitTestIntrinsic(CodeGenFunction 
,
   llvm::IntegerType *IntType =