[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1199808afa5: [clang][ConstExprEmitter] handle 
ArrayToPointerDecay ImplicitCastExpr of… (authored by nickdesaulniers).

Changed prior to commit:
  https://reviews.llvm.org/D156185?vs=543740=544125#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

Files:
  clang/lib/CodeGen/CGExprConstant.cpp


Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1127,6 +1127,11 @@
 case CK_ConstructorConversion:
   return Visit(subExpr, destType);
 
+case CK_ArrayToPointerDecay:
+  if (const auto *S = dyn_cast(subExpr))
+return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
+  return nullptr;
+
 case CK_IntToOCLSampler:
   llvm_unreachable("global sampler variables are not generated");
 
@@ -1164,7 +1169,6 @@
 case CK_CPointerToObjCPointerCast:
 case CK_BlockPointerToObjCPointerCast:
 case CK_AnyPointerToBlockPointerCast:
-case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_BaseToDerived:
 case CK_DerivedToBase:


Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1127,6 +1127,11 @@
 case CK_ConstructorConversion:
   return Visit(subExpr, destType);
 
+case CK_ArrayToPointerDecay:
+  if (const auto *S = dyn_cast(subExpr))
+return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
+  return nullptr;
+
 case CK_IntToOCLSampler:
   llvm_unreachable("global sampler variables are not generated");
 
@@ -1164,7 +1169,6 @@
 case CK_CPointerToObjCPointerCast:
 case CK_BlockPointerToObjCPointerCast:
 case CK_AnyPointerToBlockPointerCast:
-case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_BaseToDerived:
 case CK_DerivedToBase:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

I don't have any concern with this specific patch; LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D156185#4533083 , @efriedma wrote:

> No, that's taking the address of a string literal lvalue.

ah! sorry, yeah
`const char* foo = "foo";`
vs
`char foo[10] = "x";`

That AST looks like:

  `-VarDecl 0x55eaf6075ba8  col:6 foo 'char[10]' cinit
`-StringLiteral 0x55eaf6075c98  'char[10]' "x"

That case is handled today by the fast path today.  The case //I// refer to 
which is 1000% more common is not, hence this patch. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

No, that's taking the address of a string literal lvalue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D156185#4530163 , @efriedma wrote:

>> so if you were concerned with arrays of string literals, we need this patch.
>
> Ohhh that's not what I meant by StringLiterals.  I meant cases where the 
> string literal is an rvalue, like `char foo[10] = "x";`.  If it's an 
> lvalue, we go through ConstantLValueEmitter, which is still reasonably fast.

This patch //is// addressing the case of the string literal rvalue; see the 
commit description:

>>> const char* foo = "foo";


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> so if you were concerned with arrays of string literals, we need this patch.

Ohhh that's not what I meant by StringLiterals.  I meant cases where the 
string literal is an rvalue, like `char foo[10] = "x";`.  If it's an 
lvalue, we go through ConstantLValueEmitter, which is still reasonably fast.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I guess I lost focus on the "struct or array part."

Right... scalars weren't the concern for D76096 
.  They have much less overhead going through 
ExprConstant, and any evaluation of scalars isn't a regression because we 
currently don't have a codepath for that anyway.

> Some of these seem like very low hanging fruit, IMO, and probably still do 
> speed up constant evaluation.

Sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

For this case in this patch in particular:

  extern const char* bar;
  const char* foo [] = {
  "hello",
  "goodbye",
  bar
  };

  TranslationUnitDecl
  |-VarDecl  col:20 used bar 'const char *' extern
  `-VarDecl  line:2:13 foo 'const char *[3]' cinit
`-InitListExpr  'const char *[3]'
  |-ImplicitCastExpr  'const char *' 
  | `-ImplicitCastExpr  'char *' 
  |   `-StringLiteral  'char[6]' lvalue "hello"
  |-ImplicitCastExpr  'const char *' 
  | `-ImplicitCastExpr  'char *' 
  |   `-StringLiteral  'char[8]' lvalue "goodbye"
  `-ImplicitCastExpr  'const char *' 
`-DeclRefExpr  'const char *' lvalue Var 0x55b64410c270 'bar' 
'const char *'

so if you were concerned with arrays  
of string literals , we need this 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D156185#4530117 , @efriedma wrote:

> We can do a whole bunch of these, but at some point we hit diminishing 
> returns... bailing out here isn't *that* expensive.  Do you have some idea 
> how far you want to go with this?  Adding a couple obvious checks like this 
> isn't a big deal, but I don't want to build up a complete parallel 
> infrastructure for scalar expressions here.

My goal here is to land https://reviews.llvm.org/D76096. If I have to build a 
whole complete parallel infra to do so, I will.  It might even be nice to start 
converging or deleting some of the duplication.

Some of these seem like very low hanging fruit, IMO, and probably still do 
speed up constant evaluation.

My (mis?)interpretation of https://reviews.llvm.org/D76096#4523828 was "find 
places where `Evaluate()` from clang/lib/AST/ExprConstant.cpp is called instead 
of the fast path very recently committed in D151587 
.  I guess I lost focus on the "struct or 
array part."


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

We can do a whole bunch of these, but at some point we hit diminishing 
returns... bailing out here isn't *that* expensive.  Do you have some idea how 
far you want to go with this?  Adding a couple obvious checks like this isn't a 
big deal, but I don't want to build up a complete parallel infrastructure for 
scalar expressions here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156185/new/

https://reviews.llvm.org/D156185

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

[PATCH] D156185: [clang][ConstExprEmitter] handle ArrayToPointerDecay ImplicitCastExpr of StringLiterals

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider the following statement:

  const char* foo = "foo";

For the sub-AST:

  `-ImplicitCastExpr  'const char *' 
`-ImplicitCastExpr  'char *' 
  `-StringLiteral  'char[4]' lvalue "foo"

The address of the StringLiteral can be emitted as the Constant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156185

Files:
  clang/lib/CodeGen/CGExprConstant.cpp


Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1133,6 +1133,10 @@
   return CGM.EmitNullConstant(destType);
   return nullptr;
 }
+case CK_ArrayToPointerDecay:
+  if (const auto *S = dyn_cast(subExpr))
+return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
+  return nullptr;
 
 case CK_IntToOCLSampler:
   llvm_unreachable("global sampler variables are not generated");
@@ -1171,7 +1175,6 @@
 case CK_CPointerToObjCPointerCast:
 case CK_BlockPointerToObjCPointerCast:
 case CK_AnyPointerToBlockPointerCast:
-case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_BaseToDerived:
 case CK_DerivedToBase:


Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1133,6 +1133,10 @@
   return CGM.EmitNullConstant(destType);
   return nullptr;
 }
+case CK_ArrayToPointerDecay:
+  if (const auto *S = dyn_cast(subExpr))
+return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
+  return nullptr;
 
 case CK_IntToOCLSampler:
   llvm_unreachable("global sampler variables are not generated");
@@ -1171,7 +1175,6 @@
 case CK_CPointerToObjCPointerCast:
 case CK_BlockPointerToObjCPointerCast:
 case CK_AnyPointerToBlockPointerCast:
-case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_BaseToDerived:
 case CK_DerivedToBase:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits