[SystemZ][z/OS] Stricter condition for HLASM class instantiation

[anirudhp]

Summary

• A lot of lit tests simply specify the arch minus the triple. On z/OS, this could result in a scenario of some-other-triple-unknown-ibm-zos. This points to an incorrect triple + arch combo.
• To prevent this, isOSzOS change is switched in favour of isOSBinFormatGOFF.
• This is because, the GOFF format is set only if the triple is systemz and if the operating system is GOFF. And currently, there are no other architectures/os's using the GOFF file format.
• An argument could be made that the problematic tests be fixed to explicitly specify the arch-vendor-triple string, but there's a large number of these tests, and adding this stricter scope ensures that we aren't instantiating the incorrect instance of the AsmParser for other platforms when run on z/OS.

[uweigand]

Maybe it would be clearer to be explicit and write:

if (C.getTargetTriple().isSystemZ() && C.getTargetTriple().isOSzOS())

This should have the same effect but avoid the implicit assumptions.

[anirudhp]

In D103343#2789121, @uweigand wrote:

Maybe it would be clearer to be explicit and write:

if (C.getTargetTriple().isSystemZ() && C.getTargetTriple().isOSzOS())

This should have the same effect

Yes, that would be the same behaviour.

but avoid the implicit assumptions.

Are we making an implicit assumption? In the getDefaultFormat function in Triple.cpp, we have:

case Triple::systemz:
  if (T.isOSzOS())
    return Triple::GOFF;

GOFF is only set when the arch is systemz and the os is zos.

[uweigand]

In D103343#2789521, @anirudhp wrote:

In D103343#2789121, @uweigand wrote:

Maybe it would be clearer to be explicit and write:

if (C.getTargetTriple().isSystemZ() && C.getTargetTriple().isOSzOS())

This should have the same effect

Yes, that would be the same behaviour.

but avoid the implicit assumptions.

Are we making an implicit assumption? In the getDefaultFormat function in Triple.cpp, we have:

case Triple::systemz:
  if (T.isOSzOS())
    return Triple::GOFF;

GOFF is only set when the arch is systemz and the os is zos.

That's true now. The "implicit assumption" is that this will never change in the future.

[anirudhp]

• More explicit check which eliminates implicit assumptions

[anirudhp]

That's true now. The "implicit assumption" is that this will never change in the future.

Changed.

[uweigand]

LGTM, thanks!

llvm/lib/MC/MCParser/AsmParser.cpp

 
 } // end namespace MCParserUtils
 } // end namespace llvm
 
 /// Create an MCAsmParser instance.
 MCAsmParser *llvm::createMCAsmParser(SourceMgr &SM, MCContext &C,
                                      MCStreamer &Out, const MCAsmInfo &MAI,
                                      unsigned CB) {
-  if (C.getTargetTriple().isOSzOS())
+  if (C.getTargetTriple().isSystemZ() && C.getTargetTriple().isOSzOS())
     return new HLASMAsmParser(SM, C, Out, MAI, CB);
 
   return new AsmParser(SM, C, Out, MAI, CB);
 }