[PATCH] D73554: [AIX] Don't use a zero fill with a second parameter

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:550
   const char *getZeroDirective() const { return ZeroDirective; }
+  bool getZeroDirectiveSupportsNonZeroValue() const {
+    return ZeroDirectiveSupportsNonZeroValue;
----------------
Just a minor comment, is it better to name this function `ifZeroDirectiveSupportsNonZeroValue()` or `doesZeroDirectiveSupportsNonZeroValue()` since the return value is a `boolean`?


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1113
+    } else {
+      assert(isa<MCConstantExpr>(NumBytes) && "Cannot emit non-constant "
+                                               "expression lengths of "
----------------
Is `.byte` a AIX only directive? If yes, maybe we should assert OS as AIX as well?
And also is this assertion also apply to above `ZeroDirective`?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-nonzero-zerofill.ll:6
+
+; CHECK-NOT: .space  4,2
----------------
Since the intention of this patch includes `and if we can't zerofill non-zero values we just splat the .byte directives.`, we may also want to check correct format for this testcase ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73554