[PATCH] D115419: [PowerPC] Allow absolute expressions in relocations

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Thanks for the patch!



================
Comment at: llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp:365-387
+  bool isS16ImmX4() const {
+    switch (Kind) {
+    case Expression:
+      return true;
+    case Immediate:
+    case ContextImmediate:
+      return isInt<16>(getImmS16Context()) && (getImmS16Context() & 3) == 0;
----------------
These two functions look very similar except for one constant. Consider calling a new helper that shares an implementation but accepts the constant as a parameter.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCExpr.cpp:114
+    bool IsHalf16 =
+        Fixup && (unsigned)Fixup->getKind() == PPC::fixup_ppc_half16;
+    bool IsHalf16DS =
----------------
Using `Fixup->getTargetKind()` should avoid the need for the casts.


================
Comment at: llvm/test/MC/PowerPC/ppc64-abs-reloc.s:3-7
+	.text
+	.abiversion 2
+	.file	"b.c"
+	.globl	test                            # -- Begin function test
+	.p2align	4
----------------
I'm guessing these directives and the cfi directives below are unnecessary to the functionality of this test? I'd be curious if this test case could be pared down further?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115419