Bug 101666

Despite the code looking just fine, I can repro this with a shader_test:

[require]
GL ES >= 3.0
GLSL ES >= 3.00

[vertex shader]
#version 300 es

uint bitfieldExtract(uint val, int off, int size) {
    uint mask = uint((1 << size) - 1);
    return uint(val >> off) & mask;
}

void main() {
  gl_Position = vec4(bitfieldExtract(1u, 2, 3));
}

[fragment shader]
#version 300 es
out vec4 color;
void main() { color = vec4(0,1,0,1); }

[test]
link success


results in

Failed to compile VS: 0:3(6): error: A shader cannot redefine or overload built-in function `bitfieldExtract' in GLSL ES 3.00
0:9(21): error: no matching function for call to `bitfieldExtract(uint, int, int)'; candidates are:
0:9(16): error: cannot construct `vec4' from a non-numeric data type

Will require some code reading, because the builtin_functions.cpp signature seems fine. It must not be getting validated at the right times...

[And to avoid any confusion, this was on SNB, so ES 3.1 wasn't even an option.]

Aha. I see.

_mesa_glsl_has_builtin_function() needs to take a state, and only return true if sig->is_builtin_available(state). Unclear if _mesa_glsl_find_builtin_function needs to do the same.

Should be easy enough. I can hack it up tonight maybe, or someone else can take it.

I don't think you need to change any infrastructure, we just didn't add the proper built-in availability predicate to the bitfieldExtract functions in builtin_functions.cpp.  it should be trivial to fix - this is by no means a new case...

Created attachment 132398 [details]
attachment-11561-0.html

The predicate is correct, just never checked.

On Jul 2, 2017 15:08, <bugzilla-daemon@freedesktop.org> wrote:

> *Comment # 3 <https://bugs.freedesktop.org/show_bug.cgi?id=101666#c3> on
> bug 101666 <https://bugs.freedesktop.org/show_bug.cgi?id=101666> from
> Kenneth Graunke <kenneth@whitecape.org> *
>
> I don't think you need to change any infrastructure, we just didn't add the
> proper built-in availability predicate to the bitfieldExtract functions in
> builtin_functions.cpp.  it should be trivial to fix - this is by no means a new
> case...
>
> ------------------------------
> You are receiving this mail because:
>
>    - You are the assignee for the bug.
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>

Created attachment 132400 [details]
attachment-15206-0.html

To be clear, I'm saying this is broken in Mesa for *any* situation that
involves redefining a function that appears in a future GL version or ext.
There are some limitations to this, based on when the error is triggered, I
don't have the code in front of me. At least it happens for es 3.0.

On Jul 2, 2017 15:10, <bugzilla-daemon@freedesktop.org> wrote:

*Comment # 4 <https://bugs.freedesktop.org/show_bug.cgi?id=101666#c4> on
bug 101666 <https://bugs.freedesktop.org/show_bug.cgi?id=101666> from Ilia
Mirkin <imirkin@alum.mit.edu> *

The predicate is correct, just never checked.

On Jul 2, 2017 15:08, <bugzilla-daemon@freedesktop.org> wrote:

> *Comment # 3 <https://bugs.freedesktop.org/show_bug.cgi?id=101666#c3> <https://bugs.freedesktop.org/show_bug.cgi?id=101666#c3> on
> bug 101666 <https://bugs.freedesktop.org/show_bug.cgi?id=101666> <https://bugs.freedesktop.org/show_bug.cgi?id=101666> from
> Kenneth Graunke <kenneth@whitecape.org> *

>> I don't think you need to change any infrastructure, we just didn't add the
> proper built-in availability predicate to the bitfieldExtract functions in
> builtin_functions.cpp.  it should be trivial to fix - this is by no means a new
> case...
>
> ------------------------------
> You are receiving this mail because:
>>    - You are the assignee for the bug.

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

------------------------------
You are receiving this mail because:

   - You are the assignee for the bug.


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

Just to illustrate my point further (and apologies for not properly compensating for bz's unfortunate email-to-bug integration):

 $ cat foo.shader_test 
[require]
GL ES >= 3.0
GLSL ES >= 3.00

[vertex shader]
#version 300 es

uint bitfieldExtract(uint val, int off, int size) {
    uint mask = uint((1 << size) - 1);
    return uint(val >> off) & mask;
}

uint bitfieldReverse(uint bits) {
  return bits;
}

float interpolateAtCentroid(float foo) {
  return foo;
}

void main() {
  gl_Position = vec4(bitfieldExtract(1u, 2, 3));
}

[fragment shader]
#version 300 es
out vec4 color;
void main() { color = vec4(0,1,0,1); }

[test]
link success

$ bin/shader_runner_gles3 foo.shader_test -auto
Failed to compile VS: 0:3(6): error: A shader cannot redefine or overload built-in function `bitfieldExtract' in GLSL ES 3.00
0:8(6): error: A shader cannot redefine or overload built-in function `bitfieldReverse' in GLSL ES 3.00
0:12(7): error: A shader cannot redefine or overload built-in function `interpolateAtCentroid' in GLSL ES 3.00
0:17(21): error: no matching function for call to `bitfieldExtract(uint, int, int)'; candidates are:
0:17(16): error: cannot construct `vec4' from a non-numeric data type

PIGLIT: {"result": "fail" }

https://patchwork.freedesktop.org/patch/164980/

should do the trick.

Fix pushed to master:

commit 880f21f55d579fe2183255d031c23343da30f69e (origin/master, origin/HEAD)
Author: Ilia Mirkin <imirkin@alum.mit.edu>
Date:   Mon Jul 3 17:08:12 2017 -0400

    glsl: check if any of the named builtins are available first

It should be cherry-picked to the 'stable' mesa tree over time, when the next release is made. Thanks for reporting the issue!