nemanjai requested changes to this revision.
nemanjai added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5001
+ case ISD::INTRINSIC_VOID: {
+ if (N->getConstantOperandVal(1) == Intrinsic::ppc_tdw ||
+ N->getConstantOperandVal(1) == Intrinsic::ppc_tw) {
----------------
amyk wrote:
> Might be a good idea to save `N->getConstantOperandVal(1)` since it is being
> accessed quite a few times here.
+1
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5003
+ N->getConstantOperandVal(1) == Intrinsic::ppc_tw) {
+ unsigned Opcode = 0;
+ int16_t SImmOperand2;
----------------
amyk wrote:
> I see we emit TDI/TWI in 2/3 cases, so I was wondering if it make sense pull
> out setting the opcode in the second and third case to have the default
> opcode be:
> ```
> Opcode = N->getConstantOperandVal(1) == Intrinsic::ppc_tdw ? PPC::TDI
> : PPC::TWI;
> ```
> And then we just set the opcode to TD/TW in the first case?
+1
Same thing with the `Ops` vector. Pre-populate it and then only change the
operand that needs to be changed.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5012
+ isIntS16Immediate(N->getOperand(3), SImmOperand3);
+ // Will emit TD/TW if 2nd and 3rd operands are reg + reg or imm + imm
+ if (isOperand2IntS16Immediate == isOperand3IntS16Immediate) {
----------------
Nit: complete sentences please. Here and in other comments.
Also, please add a comment stating that the `imm + imm` form will be optimized
to either an unconditional trap or a nop in a later pass.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5036
+ "4th operand is not an Immediate");
+ int16_t TO = int(SImmOperand4) & 0x1F;
+ // when first and second bit of TO not same, swap them
----------------
This will be an uninitialized variable when compiled without asserts.
================
Comment at:
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-64bit-only.ll:3-7
+; RUN: --ppc-asm-full-reg-names -mcpu=pwr8 < %s | FileCheck %s
; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
-; RUN: -mcpu=pwr7 < %s | FileCheck %s
+; RUN: --ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN: --ppc-asm-full-reg-names -mcpu=pwr8 < %s | FileCheck %s
----------------
This change should just be pre-committed
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112285/new/
https://reviews.llvm.org/D112285
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
