Re: [Mesa-dev] [PATCH] glsl/glcpp: Handle hex constants with 0X prefix

[Timothy Arceri]

On 24/04/18 02:04, Vlad Golovkin wrote:

2018-04-23 3:53 GMT+03:00 Timothy Arceri :



On 20/04/18 06:08, Vlad Golovkin wrote:


GLSL 4.6 spec describes hex constant as:

hexadecimal-constant:
  0x hexadecimal-digit
  0X hexadecimal-digit
  hexadecimal-constant hexadecimal-digit

Right now if you have a shader with the following structure:

  #if 0X1 // or any hex number with the 0X prefix
  // some code
  #endif

the code between #if and #endif gets removed because the checking is
performed
only for "0x" prefix which results in strtoll being called with the base 8
and
after encountering the 'X' char the strtoll returns 0. Letting strtoll
detect
the base makes this limitation go away and also makes code easier to read.



The problem is strtoll supports much more than what GLSL allows.



Also added 1 test for uppercase hex prefix.
---
   src/compiler/glsl/glcpp/glcpp-parse.y| 9
++---
   src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c   | 5
+
   .../glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected   | 5
+
   3 files changed, 12 insertions(+), 7 deletions(-)
   create mode 100644
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
   create mode 100644
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
b/src/compiler/glsl/glcpp/glcpp-parse.y
index ccb3aa18d3..d83f99f1c7 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -462,13 +462,8 @@ control_line_error:
 integer_constant:
 INTEGER_STRING {
-   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {




As per my coment above strtoll supports much more than what GLSL allows.
Please just add strncmp($1, "0X", 2) == 0 to the if above.


Flex and Bison is not my strongest suit, so correct me if I am wrong,
but it seems that INTEGER_STRING can't contain leading whitespace
and/or leading plus/minus that strtoll supports.
As for the multitude of bases that strtoll supports, it would ignore
them and just choose between 8, 10 and 16 when base = 0. I think I
should have made it more clear in the code comment.


Your right I missed that when reading the docs. I'll add something to 
the comment and push your patch. Thanks!






That patch would have my r-b. If you send that fixed up patch I can push it
for you. Thanks for looking into this.




-   $$ = strtoll ($1 + 2, NULL, 16);
-   } else if ($1[0] == '0') {
-   $$ = strtoll ($1, NULL, 8);
-   } else {
-   $$ = strtoll ($1, NULL, 10);
-   }
+   /* let strtoll detect the base */
+   $$ = strtoll ($1, NULL, 0);
 }
   | INTEGER {
 $$ = $1;
diff --git
a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
new file mode 100644
index 00..1be9b28eb7
--- /dev/null
+++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
@@ -0,0 +1,5 @@
+#if 0x1234abcd == 0X1234abcd
+success
+#else
+failure
+#endif
diff --git
a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
new file mode 100644
index 00..4cf250f6bb
--- /dev/null
+++
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
@@ -0,0 +1,5 @@
+
+success
+
+
+




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl/glcpp: Handle hex constants with 0X prefix

[Vlad Golovkin]

2018-04-23 3:53 GMT+03:00 Timothy Arceri :
>
>
> On 20/04/18 06:08, Vlad Golovkin wrote:
>>
>> GLSL 4.6 spec describes hex constant as:
>>
>> hexadecimal-constant:
>>  0x hexadecimal-digit
>>  0X hexadecimal-digit
>>  hexadecimal-constant hexadecimal-digit
>>
>> Right now if you have a shader with the following structure:
>>
>>  #if 0X1 // or any hex number with the 0X prefix
>>  // some code
>>  #endif
>>
>> the code between #if and #endif gets removed because the checking is
>> performed
>> only for "0x" prefix which results in strtoll being called with the base 8
>> and
>> after encountering the 'X' char the strtoll returns 0. Letting strtoll
>> detect
>> the base makes this limitation go away and also makes code easier to read.
>
>
> The problem is strtoll supports much more than what GLSL allows.
>
>>
>> Also added 1 test for uppercase hex prefix.
>> ---
>>   src/compiler/glsl/glcpp/glcpp-parse.y| 9
>> ++---
>>   src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c   | 5
>> +
>>   .../glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected   | 5
>> +
>>   3 files changed, 12 insertions(+), 7 deletions(-)
>>   create mode 100644
>> src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>>   create mode 100644
>> src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>>
>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
>> b/src/compiler/glsl/glcpp/glcpp-parse.y
>> index ccb3aa18d3..d83f99f1c7 100644
>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
>> @@ -462,13 +462,8 @@ control_line_error:
>> integer_constant:
>> INTEGER_STRING {
>> -   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
>
>
>
> As per my coment above strtoll supports much more than what GLSL allows.
> Please just add strncmp($1, "0X", 2) == 0 to the if above.

Flex and Bison is not my strongest suit, so correct me if I am wrong,
but it seems that INTEGER_STRING can't contain leading whitespace
and/or leading plus/minus that strtoll supports.
As for the multitude of bases that strtoll supports, it would ignore
them and just choose between 8, 10 and 16 when base = 0. I think I
should have made it more clear in the code comment.

>
> That patch would have my r-b. If you send that fixed up patch I can push it
> for you. Thanks for looking into this.
>
>
>
>> -   $$ = strtoll ($1 + 2, NULL, 16);
>> -   } else if ($1[0] == '0') {
>> -   $$ = strtoll ($1, NULL, 8);
>> -   } else {
>> -   $$ = strtoll ($1, NULL, 10);
>> -   }
>> +   /* let strtoll detect the base */
>> +   $$ = strtoll ($1, NULL, 0);
>> }
>>   | INTEGER {
>> $$ = $1;
>> diff --git
>> a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>> new file mode 100644
>> index 00..1be9b28eb7
>> --- /dev/null
>> +++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>> @@ -0,0 +1,5 @@
>> +#if 0x1234abcd == 0X1234abcd
>> +success
>> +#else
>> +failure
>> +#endif
>> diff --git
>> a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>> new file mode 100644
>> index 00..4cf250f6bb
>> --- /dev/null
>> +++
>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>> @@ -0,0 +1,5 @@
>> +
>> +success
>> +
>> +
>> +
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl/glcpp: Handle hex constants with 0X prefix

[Timothy Arceri]

On 20/04/18 06:08, Vlad Golovkin wrote:

GLSL 4.6 spec describes hex constant as:

hexadecimal-constant:
 0x hexadecimal-digit
 0X hexadecimal-digit
 hexadecimal-constant hexadecimal-digit

Right now if you have a shader with the following structure:

 #if 0X1 // or any hex number with the 0X prefix
 // some code
 #endif

the code between #if and #endif gets removed because the checking is performed
only for "0x" prefix which results in strtoll being called with the base 8 and
after encountering the 'X' char the strtoll returns 0. Letting strtoll detect
the base makes this limitation go away and also makes code easier to read.


The problem is strtoll supports much more than what GLSL allows.



Also added 1 test for uppercase hex prefix.
---
  src/compiler/glsl/glcpp/glcpp-parse.y| 9 ++---
  src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c   | 5 +
  .../glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected   | 5 +
  3 files changed, 12 insertions(+), 7 deletions(-)
  create mode 100644 
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
  create mode 100644 
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
b/src/compiler/glsl/glcpp/glcpp-parse.y
index ccb3aa18d3..d83f99f1c7 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -462,13 +462,8 @@ control_line_error:
  
  integer_constant:

INTEGER_STRING {
-   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {



As per my coment above strtoll supports much more than what GLSL allows. 
Please just add strncmp($1, "0X", 2) == 0 to the if above.


That patch would have my r-b. If you send that fixed up patch I can push 
it for you. Thanks for looking into this.




-   $$ = strtoll ($1 + 2, NULL, 16);
-   } else if ($1[0] == '0') {
-   $$ = strtoll ($1, NULL, 8);
-   } else {
-   $$ = strtoll ($1, NULL, 10);
-   }
+   /* let strtoll detect the base */
+   $$ = strtoll ($1, NULL, 0);
}
  | INTEGER {
$$ = $1;
diff --git a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c 
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
new file mode 100644
index 00..1be9b28eb7
--- /dev/null
+++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
@@ -0,0 +1,5 @@
+#if 0x1234abcd == 0X1234abcd
+success
+#else
+failure
+#endif
diff --git 
a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected 
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
new file mode 100644
index 00..4cf250f6bb
--- /dev/null
+++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
@@ -0,0 +1,5 @@
+
+success
+
+
+


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl/glcpp: Handle hex constants with 0X prefix

[Vlad Golovkin]

GLSL 4.6 spec describes hex constant as:

hexadecimal-constant:
0x hexadecimal-digit
0X hexadecimal-digit
hexadecimal-constant hexadecimal-digit

Right now if you have a shader with the following structure:

#if 0X1 // or any hex number with the 0X prefix
// some code
#endif

the code between #if and #endif gets removed because the checking is performed
only for "0x" prefix which results in strtoll being called with the base 8 and
after encountering the 'X' char the strtoll returns 0. Letting strtoll detect
the base makes this limitation go away and also makes code easier to read.

Also added 1 test for uppercase hex prefix.
---
 src/compiler/glsl/glcpp/glcpp-parse.y| 9 ++---
 src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c   | 5 +
 .../glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected   | 5 +
 3 files changed, 12 insertions(+), 7 deletions(-)
 create mode 100644 
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
 create mode 100644 
src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
b/src/compiler/glsl/glcpp/glcpp-parse.y
index ccb3aa18d3..d83f99f1c7 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -462,13 +462,8 @@ control_line_error:
 
 integer_constant:
INTEGER_STRING {
-   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
-   $$ = strtoll ($1 + 2, NULL, 16);
-   } else if ($1[0] == '0') {
-   $$ = strtoll ($1, NULL, 8);
-   } else {
-   $$ = strtoll ($1, NULL, 10);
-   }
+   /* let strtoll detect the base */
+   $$ = strtoll ($1, NULL, 0);
}
 |  INTEGER {
$$ = $1;
diff --git a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c 
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
new file mode 100644
index 00..1be9b28eb7
--- /dev/null
+++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
@@ -0,0 +1,5 @@
+#if 0x1234abcd == 0X1234abcd
+success
+#else
+failure
+#endif
diff --git 
a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected 
b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
new file mode 100644
index 00..4cf250f6bb
--- /dev/null
+++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
@@ -0,0 +1,5 @@
+
+success
+
+
+
-- 
2.14.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev