[PATCH] D72461: [AIX][XCOFF] Supporting the ReadOnlyWithRel SectionKnd
Digger via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 13:15:16 PST 2020
DiggerLin added inline comments.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1874
+ if (Kind.isData() || Kind.isReadOnlyWithRel())
+ // TODO: We maybe put this under option control, because user may want to
+ // have read-only data with relocations placed into a read-only section by
----------------
Xiangling_L wrote:
> s/maybe/may ?
thanks,fixed
================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1802
if ((!GVKind.isCommon() && !GVKind.isBSS() && !GVKind.isData() &&
- !GVKind.isReadOnly()) ||
+ !GVKind.isReadOnly() && !GVKind.isReadOnlyWithRel()) ||
GVKind.isMergeable2ByteCString() || GVKind.isMergeable4ByteCString())
----------------
Xiangling_L wrote:
> Considering this `if` clause is going longer when we support more `GVKind`, does that make sense to make the code cleaner by having a helper function like `isValidGVKind`?
I do not think we need to add helper function here, I changed to simple condition. thank for let me to consider a simple one.
================
Comment at: llvm/test/CodeGen/PowerPC/aix-readonly-relocation.ll:1
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff --relocation-model=pic < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc64-ibm-aix-xcoff --relocation-model=pic < %s | FileCheck --check-prefix=CHECK64 %s
----------------
Xiangling_L wrote:
> I kinda feel the testcase name is a little misleading, is that better to call it `aix-readonly-with-relocation.ll`?
changed
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72461/new/
https://reviews.llvm.org/D72461
More information about the llvm-commits
mailing list