[PATCH] D97748: [SystemZ][z/OS] Add support to validate a HLASM Label.

[Anirudh Prasad via Phabricator via llvm-commits]

anirudhp added a comment.

In D97748#2598611 <https://reviews.llvm.org/D97748#2598611>, @hubert.reinterpretcast wrote:

> In D97748#2597782 <https://reviews.llvm.org/D97748#2597782>, @anirudhp wrote:
>
>> An additional point:
>> I checked the V1R6 Reference manual again and I seem to have forgotten to include a check for "National Characters" ($,@ and #). I would like to remedy this by adding two private inline functions `isHLASMAlpha` and `isHLASMAlnum` which encapsulates the checks for what is an "HLASM" alphabet, and use them in the overridden `isLabel` routine. Is this acceptable?
>
> This raises an issue having to do with the encoding used for string literals. It seems that the XL compiler processes the string like it would for emission into the object file and then the assembly is parsed as IBM-1047.
>
> For example:
>
>   %:pragma filetag("IBM-1143")
>   %:include <stdio.h>
>   int main(void) ??<
>     unsigned a = 0;
>     %:pragma convert("IBM-1026")
>     __asm__(
>     "LAB??/x5B??/x7B??/x7C AFI %0,C'A' "
>     : "+r"(a)
>     :
>     );
>     printf("%02x??/n", a);
>   ??>
>
> parses okay because 0x5B, 0x7B, and 0x7C are the encodings for the "National Characters" in IBM-1047.

Thank you for bringing this up Hubert!

For now, We don’t have a firm decision on whether we will implement/support pragma convert. I understand that if it is supported, it will be a problem since the character values for the national characters will be different based on the encoding. At a future point, when we have more information, we can come back and review whether certain applicable functions would need to take the encoding into account, or, whether it can be designed in a way such that for example, everything is encoded in UTF-8 and the emission/translation is handled elsewhere (preferably much earlier than the respective backend). For now, I'm assuming the current implementation is fine.


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

https://reviews.llvm.org/D97748