[PATCH] D114589: [DAG] Enable ISD::EXTRACT_ELEMENT SimplifyDemandedBits handling

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/fp128-bitcast-after-operation.ll:92-106
+; PPC64-P8-LE-LABEL: test_copysign:
+; PPC64-P8-LE:       # %bb.0: # %entry
+; PPC64-P8-LE-NEXT:    li 3, 16399
+; PPC64-P8-LE-NEXT:    li 4, 3019
+; PPC64-P8-LE-NEXT:    rldic 3, 3, 48, 1
+; PPC64-P8-LE-NEXT:    rldic 4, 4, 52, 0
+; PPC64-P8-LE-NEXT:    blr
----------------
RKSimon wrote:
> nemanjai wrote:
> > RKSimon wrote:
> > > lebedev.ri wrote:
> > > > nemanjai wrote:
> > > > > Something seems very wrong here. We seem to simply be materializing a pair of constants which isn't what the test case does.
> > > > > Before this change, we had:
> > > > > ```
> > > > > t4: f64,ch = CopyFromReg t0, Register:f64 %1
> > > > > t2: f64,ch = CopyFromReg t0, Register:f64 %0
> > > > > t5: ppcf128 = build_pair t4, t2
> > > > > t18: i128 = bitcast t5
> > > > > t21: i64 = extract_element t20, Constant:i64<0>
> > > > > t23: i64 = and t21, Constant:i64<-9223372036854775808>
> > > > > ...
> > > > > ```
> > > > > And with this change at the same stage, we have:
> > > > > ```
> > > > > t4: f64,ch = CopyFromReg t0, Register:f64 %1
> > > > > t5: ppcf128 = build_pair t4, undef:f64
> > > > > t18: i128 = bitcast t5
> > > > > t21: i64 = extract_element t18, Constant:i64<0>
> > > > > t23: i64 = and t21, Constant:i64<-9223372036854775808>
> > > > > ```
> > > > > Then type legalization turns `t23` into:
> > > > > ```
> > > > > t23: i64 = and undef:i64, Constant:i64<-9223372036854775808>
> > > > > ```
> > > > > thereby reducing the entire function to:
> > > > > ```
> > > > > t14: ch,glue = CopyToReg t0, Register:i64 $x3, Constant:i64<4615908143078047744>
> > > > > t16: ch,glue = CopyToReg t14, Register:i64 $x4, Constant:i64<-4850376798678024192>, t14:1
> > > > > ```
> > > > What is t20 there?
> > > That is strange - SimplifyDemandedBits does have a tendency to exploit any weaknesses in other combines, maybe the ppc_fp128 handling in DAGCombiner::visitBITCAST is doing something odd?
> > > What is t20 there?
> > ```
> > t19: i128 = bitcast ConstantFP:ppcf128<APFloat(250808707454666313773153002829725564928)>
> > t20: i128 = xor t19, t18
> > ```
> Why is this is only tested with a constant ppcf128? 
This test case was added 6 years ago in 325e8d06dcfb32c636ea30c0c5b3ab6bdf020f56. I don't remember why the only test added gets the magnitude from a constant and the sign from a variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114589