[PATCH] D86689: [DAGCombine] Don't delete the node if it has uses immediately
Qing Shan Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 17:54:34 PDT 2020
steven.zhang added inline comments.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5823
Cost = CostX;
+ SDValue N = DAG.getNode(ISD::FSUB, DL, VT, NegX, Y, Flags);
RemoveDeadNode(NegY);
----------------
steven.zhang wrote:
> spatel wrote:
> > I think it would be clearer and less code duplication to put the 'if' check into the lambda, so something like:
> > auto RemoveDeadNode = [&](SDValue N, SDValue Other) {
> > if (N && N.getNode()->use_empty() && N != Other)
> > DAG.RemoveDeadNode(N.getNode());
> > };
> >
> > Then call it with something like this:
> > RemoveDeadNode(NegX, NegY);
> >
> we need also check NegX != NegZ...
So, I think, move it after the getNode() will be more safe and make RemoveDeadNode() semantics clean. Otherwise, we have to check NegY != NegX and NegY != NegZ for FMA.
================
Comment at: llvm/test/CodeGen/PowerPC/fneg.ll:56-62
+ %_val_rw_ = load float, float* undef, align 4
+ %_val_w_ = load float, float* undef, align 4
+ %_sub_tmp = fsub fast float 1.000000e+00, %_val_w_
+ %_conv25 = fpext float %_sub_tmp to double
+ %_expi_result = call fast double @llvm.powi.f64(double %_conv25, i32 3)
+ %_conv26 = fptrunc double %_expi_result to float
+ store float %_conv26, float* undef, align 4
----------------
spatel wrote:
> We should reduce the test to avoid UB and extra operations, so something like this should work?
>
> ```
> define double @fneg_no_ice(float %x) {
> %y = fsub fast float 1.0, %x
> %e = fpext float %y to double
> %e2 = fmul double %e, %e
> %e3 = fmul double %e, %e2
> ret double %e3
> }
>
> ```
Good point. Yes, it works.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86689/new/
https://reviews.llvm.org/D86689
More information about the llvm-commits
mailing list