[PATCH] D116915: [DAGCombiner][AArch64] Enhance to support for scalar CSINC

[Allen zhong via Phabricator via llvm-commits]

Allen added a comment.

Now, I can make true **the x86 debian fail** is brought in by other patch already submited in the upstream,
as it still have the same fail at the commit node **Diff 10	403452**	after rolling back the change of delete the condition !RHS.hasOneUse(),
and it is successfully at the commit node **Diff 8	402198**	, which has same change.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14630
+
+  if (!CSel.hasOneUse() || !RHS.hasOneUse())
+    return SDValue();
----------------
dmgreen wrote:
> Allen wrote:
> > dmgreen wrote:
> > > I don't think it matters if the RHS has more than 1 use, it should still be OK to transform as we only use the value in another expression.
> > I had delete the condition !RHS.hasOneUse(), but recently  the upstream may has some regression, it doen't finish the precommit build checking.  so may  I need wait a moment  to figure out that issue ?
> Hmm. That sounds suspicious. What was the problem? That would usually imply an infinite loop in DAG combine, but I'm not sure how `csinc(add(x, y), x, cc)` could get back to `add(x, csel(y, 1, cc))`. Or why the uses of y matter in that case.
> 
> I was about to OK the revision, but it would be good to make sure that nothing problematic is happening.
hi, dmgreen
   In fact, I can't reproduce the failure base on my local building with delete the condition !RHS.hasOneUse(), so it may be not this change cause the regression, as you can see the other person's commit don't also pass building recently.

   Just to be sure, do you think it is ok to roll back the change of delete the condition !RHS.hasOneUse(), and submit it firstly ? I'll resubmit the change of delete the condition !RHS.hasOneUse() after the upstream is stable with double checking.


The building log is finally came out record in https://reviews.llvm.org/harbormaster/build/215411/, and detail info can be found in [[ https://buildkite.com/llvm-project/premerge-checks/builds/76065#e9958d49-b20c-4465-ba63-7c9372fb5b4f | x86 debian fail ]]
Failed Tests (9):
  libarcher :: races/critical-unrelated.c
  libarcher :: races/lock-nested-unrelated.c
  libarcher :: races/lock-unrelated.c
  libarcher :: races/parallel-simple.c
  libarcher :: races/task-dependency.c
  libarcher :: races/task-taskgroup-unrelated.c
  libarcher :: races/task-taskwait-nested.c
  libarcher :: races/task-two.c
  libarcher :: task/task_late_fulfill.c


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

https://reviews.llvm.org/D116915