[PATCH] D59356: [SelectionDAGBuilder] Use accumulator value in VECREDUCE_FADD/FMUL

Thu Mar 14 14:47:32 PDT 2019

RKSimon added a comment.

In D59356#1429865 <https://reviews.llvm.org/D59356#1429865>, @aemerson wrote:

> In D59356#1429581 <https://reviews.llvm.org/D59356#1429581>, @sdesmalen wrote:
>
> > @nikic thanks for pointing me to that discussion! I clearly misread the LangRef for this change :)
> >  I agree it makes more sense to change these intrinsics and to make its accumulator argument always relevant (regardless of what flags are set) and AutoUpgrading older IR. Given that I'm currently working on this, I'd be happy to move this forward with patches and a proposal/discussion on the mailing list to change the experimental reduction intrinsics. @aemerson you expressed an intention to work on it later this year, do you have any objection to me moving forward with this now?
>
>
> If you're going down the auto-upgrade route then I suggest proposing that we promote these from experimental to first class intrinsics. That way you can auto-upgrade form one intrinsic to another without any risk of breaking older code (i.e. you can't just start using an accumulator arg that before could be unused and therefore undef).

I'm not sure I agree - we shouldn't drop the experimental state unless we're certain the intrinsic is not going to need to be further tweaked in the future. I still think not including an accumulator argument at all would be for the best.

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

https://reviews.llvm.org/D59356