[PATCH] D138107: [AArch64][MachineCombiner] Update isAssociativeAndCommutative

[Marco Elver via Phabricator via llvm-commits]

melver added a comment.

In D138107#3930158 <https://reviews.llvm.org/D138107#3930158>, @kawashima-fj wrote:

> @melver
> As shown in the change of `llvm/test/CodeGen/AArch64/GlobalISel/arm64-pcsections.ll`, the Machine InstCombiner pass removes `pcsections` metadata when replacing `MachineInstr`. I'm not sure it should nor not.
>
> Essentially, the Machine InstCombiner pass changes MIR
>
>   $5 = ADD $1, $2, pcsections !0
>   $6 = ADD $5, $3, pcsections !0
>   $7 = ADD $6, $4, pcsections !0
>
> to
>
>   $5 = ADD $1, $2, pcsections !0
>   $6 = ADD $3, $4
>   $7 = ADD $5, $6
>
> Should `pcsections !0` in `$6` and `$7` be preserved? If so, probablly code to create new MachineInstr <https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/TargetInstrInfo.cpp#L871> should be updated.

Yes, because the `add` instructions in LLVM IR have `!pcsections` attached to them.

It should be enough to use `BuildMI(*MF, MIMetadata(Prev), TII->get(Opcode), NewVR) ...` for `Prev` and `BuildMI(*MF, MIMetadata(Root), TII->get(Opcode), RegC)...` for `Root`.

Thanks for pointing it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138107