<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 30/04/18 20:51, Jason Ekstrand
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAOFGe97K8ky=vCDO9AmQnNLP80AVrA=TDs84wneT2-GQ2gfpKg@mail.gmail.com">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">On Mon, Apr 30, 2018 at 1:46 AM,
            Samuel Iglesias Gonsálvez <span dir="ltr"><<a
                href="mailto:siglesias@igalia.com" target="_blank"
                moz-do-not-send="true">siglesias@igalia.com</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div text="#000000" bgcolor="#FFFFFF">
                <div>
                  <div class="h5"> On 26/04/18 18:14, Jason Ekstrand
                    wrote:<br>
                    <blockquote type="cite">
                      <div dir="ltr"><br>
                        <div class="gmail_extra"><br>
                          <div class="gmail_quote">On Thu, Apr 26, 2018
                            at 2:24 AM, Samuel Iglesias Gonsálvez <span
                              dir="ltr"><<a
                                href="mailto:siglesias@igalia.com"
                                target="_blank" moz-do-not-send="true">siglesias@igalia.com</a>></span>
                            wrote:<br>
                            <blockquote class="gmail_quote"
                              style="margin:0 0 0 .8ex;border-left:1px
                              #ccc solid;padding-left:1ex">SPIR-V allows
                              to define the shift operand for shift
                              opcodes with<br>
                              a bit-size different than 32 bits, but in
                              NIR the opcodes have<br>
                              that limitation. As agreed in the mailing
                              list, this patch adds<br>
                              a conversion to 32 bits to fix this.<br>
                              <br>
                              For more info, see:<br>
                              <br>
                              <a
href="https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html"
                                rel="noreferrer" target="_blank"
                                moz-do-not-send="true">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2018-April/1<wbr>93026.html</a><br>
                              <br>
                              Signed-off-by: Samuel Iglesias Gonsálvez
                              <<a href="mailto:siglesias@igalia.com"
                                target="_blank" moz-do-not-send="true">siglesias@igalia.com</a>><br>
                              ---<br>
                               src/compiler/spirv/vtn_alu.c | 48
                              ++++++++++++++++++++++++++++++<wbr>++++++++++++++<br>
                               1 file changed, 48 insertions(+)<br>
                              <br>
                              diff --git a/src/compiler/spirv/vtn_alu.c
                              b/src/compiler/spirv/vtn_alu.c<br>
                              index 6f3b82cd5c3..a1654b20273 100644<br>
                              --- a/src/compiler/spirv/vtn_alu.c<br>
                              +++ b/src/compiler/spirv/vtn_alu.c<br>
                              @@ -640,6 +640,54 @@ vtn_handle_alu(struct
                              vtn_builder *b, SpvOp opcode,<br>
                                     break;<br>
                                  }<br>
                              <br>
                              +   case SpvOpBitFieldInsert: {<br>
                              +      if (src[2]->bit_size != 32) {<br>
                              +         /* Convert the Shift operand to
                              32 bits, which is the bitsize<br>
                              +          * supported by the NIR
                              instruction. See discussion here:<br>
                              +          *<br>
                              +          * <a
href="https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html"
                                rel="noreferrer" target="_blank"
                                moz-do-not-send="true">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2018-April/1<wbr>93026.html</a><br>
                              +          */<br>
                              +         src[2] =
                              nir_build_alu(&b->nb, nir_op_u2u32,
                              src[2], NULL, NULL, NULL);<br>
                              +      }<br>
                              +      if (src[3]->bit_size != 32) {<br>
                              +         /* Convert the Shift operand to
                              32 bits, which is the bitsize<br>
                              +          * supported by the NIR
                              instruction. See discussion here:<br>
                              +          *<br>
                              +          * <a
href="https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html"
                                rel="noreferrer" target="_blank"
                                moz-do-not-send="true">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2018-April/1<wbr>93026.html</a><br>
                              +          */<br>
                              +         src[3] =
                              nir_build_alu(&b->nb, nir_op_u2u32,
                              src[3], NULL, NULL, NULL);<br>
                              +      }<br>
                              +      val->ssa->def =
                              nir_build_alu(&b->nb,
                              nir_op_bitfield_insert, src[0], src[1],
                              src[2], src[3]);<br>
                            </blockquote>
                            <div><br>
                            </div>
                            <div>Just use nir_bitfield_insert here<br>
                            </div>
                            <div> </div>
                            <blockquote class="gmail_quote"
                              style="margin:0 0 0 .8ex;border-left:1px
                              #ccc solid;padding-left:1ex"> +     
                              break;<br>
                              +   }<br>
                              +<br>
                              +   case SpvOpBitFieldSExtract:<br>
                              +   case SpvOpBitFieldUExtract: {<br>
                              +      bool swap;<br>
                              +      unsigned src_bit_size =
                              glsl_get_bit_size(vtn_src[0]-><wbr>type);<br>
                              +      unsigned dst_bit_size =
                              glsl_get_bit_size(type);<br>
                              +      nir_op op =
                              vtn_nir_alu_op_for_spirv_opcod<wbr>e(b,
                              opcode, &swap,<br>
                              +                                         
                                      src_bit_size, dst_bit_size);<br>
                              +      if (src[1]->bit_size != 32) {<br>
                              +         /* Convert the Shift operand to
                              32 bits, which is the bitsize<br>
                              +          * supported by the NIR
                              instruction. See discussion here:<br>
                              +          *<br>
                              +          * <a
href="https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html"
                                rel="noreferrer" target="_blank"
                                moz-do-not-send="true">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2018-April/1<wbr>93026.html</a><br>
                              +          */<br>
                              +         src[1] =
                              nir_build_alu(&b->nb, nir_op_u2u32,
                              src[1], NULL, NULL, NULL);<br>
                              +      }<br>
                              +      if (src[2]->bit_size != 32) {<br>
                              +         /* Convert the Shift operand to
                              32 bits, which is the bitsize<br>
                              +          * supported by the NIR
                              instruction. See discussion here:<br>
                              +          *<br>
                              +          * <a
href="https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html"
                                rel="noreferrer" target="_blank"
                                moz-do-not-send="true">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2018-April/1<wbr>93026.html</a><br>
                              +          */<br>
                              +         src[2] =
                              nir_build_alu(&b->nb, nir_op_u2u32,
                              src[2], NULL, NULL, NULL);<br>
                              +      }<br>
                              +      val->ssa->def =
                              nir_build_alu(&b->nb, op, src[0],
                              src[1], src[2], src[3]);<br>
                              +      break;<br>
                            </blockquote>
                            <div><br>
                            </div>
                            <div>I'm wondering if we don't want to just
                              do something such as<br>
                              <br>
                            </div>
                            <div>for (unsigned i = 0; i <
                              nir_op_infos[op].num_inputs; i++) {<br>
                            </div>
                            <div>   src_bit_size =
                              nir_alu_type_get_get_type_<wbr>size(nir_op_infos[op].input_<wbr>types[i]);<br>
                            </div>
                            <div>   if (src_bit_size &&
                              src_bit_size != src[i]->bit_size) {<br>
                            </div>
                            <div>      /* Comment goes here */<br>
                            </div>
                            <div>      assert(src_bit_size == 32);<br>
                            </div>
                            <div>      assert(op == nir_op_ushr || op ==
                              ...);<br>
                            </div>
                            <div>      src[i] = nir_u2u32(&b->nb,
                              src[i]);<br>
                            </div>
                            <div>   }<br>
                            </div>
                            <div>}<br>
                              <br>
                            </div>
                            <div>And then we can cover all of these
                              cases in one go.<br>
                            </div>
                            <div> </div>
                          </div>
                        </div>
                      </div>
                    </blockquote>
                    <br>
                  </div>
                </div>
                Right, but I don't want to convert all of them. The
                starting value for the loop counter should be 1 for some
                ops and 2 for others, so we don't convert the operand
                that has the value that we will operate.<br>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>I believe the sources you don't want to convert will
              have src_bit_size == 0.<br>
              <br>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    Right. Just sent a new version of the patch.<br>
    <br>
    Thanks,<br>
    <br>
    Sam<br>
    <br>
    <blockquote type="cite"
cite="mid:CAOFGe97K8ky=vCDO9AmQnNLP80AVrA=TDs84wneT2-GQ2gfpKg@mail.gmail.com">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div>--Jason<br>
            </div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div text="#000000" bgcolor="#FFFFFF"> I'm going to write
                a patch for that. Thanks for the suggestions.<br>
                <br>
                Sam<span class=""><br>
                  <br>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div class="gmail_extra">
                        <div class="gmail_quote">
                          <blockquote class="gmail_quote"
                            style="margin:0 0 0 .8ex;border-left:1px
                            #ccc solid;padding-left:1ex"> +   }<br>
                            +<br>
                                case SpvOpShiftLeftLogical:<br>
                                case SpvOpShiftRightArithmetic:<br>
                                case SpvOpShiftRightLogical: {<br>
                            <span class="m_-480218408964756242HOEnZb"><font
                                color="#888888">-- <br>
                                2.14.1<br>
                                <br>
                                ______________________________<wbr>_________________<br>
                                mesa-dev mailing list<br>
                                <a
                                  href="mailto:mesa-dev@lists.freedesktop.org"
                                  target="_blank" moz-do-not-send="true">mesa-dev@lists.freedesktop.org</a><br>
                                <a
                                  href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev"
                                  rel="noreferrer" target="_blank"
                                  moz-do-not-send="true">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
                              </font></span></blockquote>
                        </div>
                        <br>
                      </div>
                    </div>
                  </blockquote>
                  <br>
                </span></div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>