<div dir="ltr"><div dir="ltr">On Mon, Oct 4, 2021 at 8:05 AM Jonas Paulsson <<a href="mailto:paulsson@linux.vnet.ibm.com">paulsson@linux.vnet.ibm.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div>
    <p>Hi Reid,</p>
    <blockquote type="cite">
      <div dir="ltr">
        <div>I don't think it would be reasonable for the SystemZ
          backend to abort every time it is asked to code generate an
          i32 argument that lacks an attribute. There are simply too
          many IR producers (frontends and instrumentation tools) that
          will pass unannotated i32 values to calls. It's not feasible
          to fix them all, and aborting just means these producers will
          have to be updated whenever they are ported to SystemZ or any
          other target adopting the same practice.</div>
        <div><br>
        </div>
        <div>I think it would instead be reasonable to pick a safer
          default for the target, like always zero extending unannotated
          parameters on both the caller side and the callee side. This
          is less efficient than only extending once in the caller, but
          it makes it much easier to retarget an IR producer over to
          SystemZ. It makes the attributes into performance
          improvements, not correctness requirements. I suspect this
          will not actually break passing a 4 byte struct, it just makes
          it less efficient.</div>
        <div><br>
        </div>
      </div>
    </blockquote>
    <p>As Uli explained, this is not an optimization but actually really
      necessary in order to perform the right extension. I don't think
      there is any way around fixing this the slow and arduous way :-/</p></div></blockquote><div>Well, the annotation isn't strictly necessary. The backend can choose to emit an extension for short integer arguments in the absence of annotations. The value would be to reduce the number of SystemZ-specific changes to IR producers. It would be a shame if i32 arguments just didn't work out of the box on SystemZ. That will be an ongoing source of IR producer bugs.<br></div><div><br></div><div>In any case, I don't feel like my argument is all that compelling. If you and the SystemZ owners feel like this is the best plan, go for it.</div><div><br></div><div>The same problem exists for i16 and i8 arguments on x86, BTW: <a href="https://gcc.godbolt.org/z/7dds6sfv8">https://gcc.godbolt.org/z/7dds6sfv8</a> These are much less common argument types, but you might consider expanding the scope of this proposal.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>
    <blockquote type="cite">
      <div dir="ltr">
        <div>As for the suggestion to add an explicit structarg
          attribute, can this be generalized to an attribute that marks
          the high bits as undef? So, IR producers will have a portable
          way to explicitly request the current behavior. This is kind
          of like `anyext`, but not really, more like `undefext`,
          `uext`, `hiundef` or `calleemustextend`.</div>
        <div><br>
        </div>
        <div>What do you think?</div>
      </div>
    </blockquote>
    <p>I agree that some more general attribute could work as well to
      mean for a parameter that it is not an integer value and no
      extension is therefore required per the ABI. The important thing
      about the name is that whoever builds the call instruction needs
      to be aware that it is only for non-integer values - the other
      trap to fall into I guess might be that 'anyext' could be added
      carelessly on integers when it seems that it might not matter,
      even when it really does per the ABI... :-)  Maybe the name
      'anyext' would make people think that it will be extended one way
      or the other, which is not true. I think I like your 'hiundef'
      better... Or maybe 'nonint', or 'noext'?<br>
    </p>
    <p>Do you agree we need to do this, and what would the next step be
      towards getting this in place? If we decide on the attribute to
      use, I could implement the assertion and start filing bugs and
      hopefully get some help with them as well... Even though this may
      not be "feasible", I hope this would eventually lead to having
      this working correctly, or is there any reason not?</p></div></blockquote><div>I think I like `noext`, it fits well with the existing attributes.<br></div><div><br></div><div>As for next steps, we need to settle on the name. If you like `noext`, send a patch, CC some other folks and see if they like the name.</div></div></div>