[PATCH] D85377: [PowerPC] Add option to control PCRel GOT indirect linker optimization

[Victor Huang via Phabricator via llvm-commits]

NeHuang added a comment.

Overall LGTM. I only have some nits comment/question for the test case and clang-format related issue.



================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:42
+static cl::opt<bool>
+EnablePCRelLinkerOpt("ppc-pcrel-linker-opt", cl::Hidden, cl::init(true),
+                     cl::desc("enable PC Relative linker optimization"));
----------------
nit: clang-format


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt-option.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=future -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
+; RUN:   < %s | FileCheck %s --check-prefix=DEFAULT
----------------
nit: can we use `-mcpu=pwr10` here?


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt-option.ll:6
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=future -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
+; RUN:   -ppc-pcrel-linker-opt=true < %s | FileCheck %s --check-prefix=ON
----------------
ditto 


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt-option.ll:9
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=future -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
+; RUN:   -ppc-pcrel-linker-opt=false < %s | FileCheck %s --check-prefix=OFF
----------------
ditto


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt-option.ll:14
+
+define dso_local i8 @Read8() local_unnamed_addr #0 {
+; DEFAULT-LABEL: Read8:
----------------
nit: Do we need to keep `#0` and `attributes #0 = { nounwind }` below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85377