[llvm] r355124 - bpf: improve dead Defs check for XADD

Thu Feb 28 11:21:51 PST 2019

Test? (Was this reviewed somewhere?)

On Thu, Feb 28, 2019 at 10:19 PM Jiong Wang via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: jiwang
> Date: Thu Feb 28 11:20:26 2019
> New Revision: 355124
>
> URL: http://llvm.org/viewvc/llvm-project?rev=355124&view=rev
> Log:
> bpf: improve dead Defs check for XADD
>
> BPF XADD semantics require all Defs of XADD are dead, meaning any result of
> XADD insn is not used.
>
> However, BPF backend hasn't enabled sub-register liveness track, so when
> the source and destination operands of XADD are GPR32, there is no
> sub-register dead info. If we rely on the generic
> MachineInstr::allDefsAreDead, then we will raise false alarm on GPR32 Def.
> This was fine as there was no sub-register code-gen support for XADD which
> will be added by the next patch.
>
> To support GPR32 Def, ideally we could just enable sub-registr liveness
> track on BPF backend, then allDefsAreDead could work on GPR32 Def. This
> requires implementing TargetSubtargetInfo::enableSubRegLiveness on BPF.
>
> However, sub-register liveness tracking module inside LLVM is actually
> designed for the situation where one register could be split into more
> than one sub-registers for which case each sub-register could have their
> own liveness and kill one of them doesn't kill others. So, tracking
> liveness for each make sense.
>
> For BPF, each 64-bit register could only have one 32-bit sub-register. This
> is exactly the case which LLVM think brings no benefits for doing
> sub-register tracking, because the live range of sub-register must always
> equal to its parent register, therefore liveness tracking is disabled even
> the back-end has implemented enableSubRegLiveness. The detailed information
> is at r232695:
>
>   Author: Matthias Braun <matze at braunis.de>
>   Date:   Thu Mar 19 00:21:58 2015 +0000
>   Do not track subregister liveness when it brings no benefits
>
> Hence, for BPF, we enhance MachineInstr::allDefsAreDead. Given the solo
> sub-register always has the same liveness as its parent register, LLVM is
> already attaching a implicit 64-bit register Def whenever the there is
> a sub-register Def. The liveness of the implicit 64-bit Def is available.
> For example, for "lock *(u32 *)(r0 + 4) += w9", the MachineOperand info
> could be:
>
>   $w9 = XADDW32 killed $r0, 4, $w9(tied-def 0),
>                        implicit killed $r9, implicit-def dead $r9
>
> Even though w9 is not marked as Dead, the parent register r9 is marked as
> Dead correctly, and it is safe to use such information or our purpose.
>
> v1 -> v2:
>  - Simplified code logic inside hasLiveDefs. (Yonghong)
>
> Acked-by: Yonghong Song <yhs at fb.com>
> Signed-off-by: Jiong Wang <jiong.wang at netronome.com>
>
>
> Modified:
>     llvm/trunk/lib/Target/BPF/BPFMIChecking.cpp
>
> Modified: llvm/trunk/lib/Target/BPF/BPFMIChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/BPF/BPFMIChecking.cpp?rev=355124&r1=355123&r2=355124&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/BPF/BPFMIChecking.cpp (original)
> +++ llvm/trunk/lib/Target/BPF/BPFMIChecking.cpp Thu Feb 28 11:20:26 2019
> @@ -61,6 +61,97 @@ void BPFMIPreEmitChecking::initialize(Ma
>    LLVM_DEBUG(dbgs() << "*** BPF PreEmit checking pass ***\n\n");
>  }
>
> +// Make sure all Defs of XADD are dead, meaning any result of XADD insn is not
> +// used.
> +//
> +// NOTE: BPF backend hasn't enabled sub-register liveness track, so when the
> +// source and destination operands of XADD are GPR32, there is no sub-register
> +// dead info. If we rely on the generic MachineInstr::allDefsAreDead, then we
> +// will raise false alarm on GPR32 Def.
> +//
> +// To support GPR32 Def, ideally we could just enable sub-registr liveness track
> +// on BPF backend, then allDefsAreDead could work on GPR32 Def. This requires
> +// implementing TargetSubtargetInfo::enableSubRegLiveness on BPF.
> +//
> +// However, sub-register liveness tracking module inside LLVM is actually
> +// designed for the situation where one register could be split into more than
> +// one sub-registers for which case each sub-register could have their own
> +// liveness and kill one of them doesn't kill others. So, tracking liveness for
> +// each make sense.
> +//
> +// For BPF, each 64-bit register could only have one 32-bit sub-register. This
> +// is exactly the case which LLVM think brings no benefits for doing
> +// sub-register tracking, because the live range of sub-register must always
> +// equal to its parent register, therefore liveness tracking is disabled even
> +// the back-end has implemented enableSubRegLiveness. The detailed information
> +// is at r232695:
> +//
> +//   Author: Matthias Braun <matze at braunis.de>
> +//   Date:   Thu Mar 19 00:21:58 2015 +0000
> +//   Do not track subregister liveness when it brings no benefits
> +//
> +// Hence, for BPF, we enhance MachineInstr::allDefsAreDead. Given the solo
> +// sub-register always has the same liveness as its parent register, LLVM is
> +// already attaching a implicit 64-bit register Def whenever the there is
> +// a sub-register Def. The liveness of the implicit 64-bit Def is available.
> +// For example, for "lock *(u32 *)(r0 + 4) += w9", the MachineOperand info could
> +// be:
> +//
> +//   $w9 = XADDW32 killed $r0, 4, $w9(tied-def 0),
> +//                        implicit killed $r9, implicit-def dead $r9
> +//
> +// Even though w9 is not marked as Dead, the parent register r9 is marked as
> +// Dead correctly, and it is safe to use such information or our purpose.
> +static bool hasLiveDefs(const MachineInstr &MI, const TargetRegisterInfo *TRI) {
> +  const MCRegisterClass *GPR64RegClass =
> +    &BPFMCRegisterClasses[BPF::GPRRegClassID];
> +  std::vector<unsigned> GPR32LiveDefs;
> +  std::vector<unsigned> GPR64DeadDefs;
> +
> +  for (const MachineOperand &MO : MI.operands()) {
> +    bool RegIsGPR64;
> +
> +    if (!MO.isReg() || MO.isUse())
> +      continue;
> +
> +    RegIsGPR64 = GPR64RegClass->contains(MO.getReg());
> +    if (!MO.isDead()) {
> +      // It is a GPR64 live Def, we are sure it is live. */
> +      if (RegIsGPR64)
> +        return true;
> +      // It is a GPR32 live Def, we are unsure whether it is really dead due to
> +      // no sub-register liveness tracking. Push it to vector for deferred
> +      // check.
> +      GPR32LiveDefs.push_back(MO.getReg());
> +      continue;
> +    }
> +
> +    // Record any GPR64 dead Def as some unmarked GPR32 could be alias of its
> +    // low 32-bit.
> +    if (RegIsGPR64)
> +      GPR64DeadDefs.push_back(MO.getReg());
> +  }
> +
> +  // No GPR32 live Def, safe to return false.
> +  if (GPR32LiveDefs.empty())
> +    return false;
> +
> +  // No GPR64 dead Def, so all those GPR32 live Def can't have alias, therefore
> +  // must be truely live, safe to return true.
> +  if (GPR64DeadDefs.empty())
> +    return true;
> +
> +  // Otherwise, return true if any aliased SuperReg of GPR32 is not dead.
> +  std::vector<unsigned>::iterator search_begin = GPR64DeadDefs.begin();
> +  std::vector<unsigned>::iterator search_end = GPR64DeadDefs.end();
> +  for (auto I : GPR32LiveDefs)
> +    for (MCSuperRegIterator SR(I, TRI); SR.isValid(); ++SR)
> +       if (std::find(search_begin, search_end, *SR) == search_end)
> +         return true;
> +
> +  return false;
> +}
> +
>  void BPFMIPreEmitChecking::checkingIllegalXADD(void) {
>    for (MachineBasicBlock &MBB : *MF) {
>      for (MachineInstr &MI : MBB) {
> @@ -68,7 +159,7 @@ void BPFMIPreEmitChecking::checkingIlleg
>          continue;
>
>        LLVM_DEBUG(MI.dump());
> -      if (!MI.allDefsAreDead()) {
> +      if (hasLiveDefs(MI, TRI)) {
>          DebugLoc Empty;
>          const DebugLoc &DL = MI.getDebugLoc();
>          if (DL != Empty)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits