[SelectionDAGBuilder] Fix ISD::FREEZE creation for structs with fields of different types.

[craig.topper]

Summary

The previous code used the type of the first field for the VT
passed to getNode for every field.

I've based the implementation here off what is done in visitSelect
as it removes the need to special case aggregates.

[aqjune]

Looks good to me, but maybe someone else who knows SelDag well should review this as well..?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

10542

Realized that the original code had a problem with a nested struct type.
It would be great if there is a test for freeze with nested struct type.

[aqjune]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

10542

Sorry, the original code seems to successfully deal with a nested struct type too.

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

10542

I think it still has a bug if the types of the other fields in the struct don't match the first type. For example this triggers an assert. (and not the one i just added..

define i64 @freeze_anonstruct() {

%y1 = freeze {i32, {i64}} undef
%v1 = extractvalue {i32, {i64}} %y1, 0
%v2 = extractvalue {i32, {i64}} %y1, 1, 0
%v3 = zext i32 %v1 to i64
%t1 = add i64 %v3, %v2
ret i64 %t1

}

[aqjune]

In D77093#1951668, @aqjune wrote:

Looks good to me, but maybe someone else who knows SelDag well should review this as well..?

Oh I meant just anyone other than me, including people who were already added as reviewers. :)

[aqjune]

Clarify that the change seems good to me

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (NumClusters > 3 && TM.getOptLevel() != CodeGenOpt::None &&
       continue;
     }
 
     lowerWorkItem(W, SI.getCondition(), SwitchMBB, DefaultMBB);
   }
 }
 
 void SelectionDAGBuilder::visitFreeze(const FreezeInst &I) {
-  SDNodeFlags Flags;
+  SmallVector<EVT, 4> ValueVTs;
+  ComputeValueVTs(DAG.getTargetLoweringInfo(), DAG.getDataLayout(), I.getType(),
+                  ValueVTs);
+  unsigned NumValues = ValueVTs.size();
+  if (NumValues == 0) return;
 
+  SmallVector<SDValue, 4> Values(NumValues);
   SDValue Op = getValue(I.getOperand(0));
-  if (I.getOperand(0)->getType()->isAggregateType()) {
-    EVT VT = Op.getValueType();
-    SmallVector<SDValue, 1> Values;
-    for (unsigned i = 0; i < Op.getNumOperands(); ++i) {
-      SDValue Arg(Op.getNode(), i);
-      SDValue UnNodeValue = DAG.getNode(ISD::FREEZE, getCurSDLoc(), VT, Arg, Flags);
-      Values.push_back(UnNodeValue);
-    }
-    SDValue MergedValue = DAG.getMergeValues(Values, getCurSDLoc());
-    setValue(&I, MergedValue);
-  } else {
-    SDValue UnNodeValue = DAG.getNode(ISD::FREEZE, getCurSDLoc(), Op.getValueType(),
-                                      Op, Flags);
-    setValue(&I, UnNodeValue);
-  }
+
+  for (unsigned i = 0; i != NumValues; ++i)
+    Values[i] = DAG.getNode(ISD::FREEZE, getCurSDLoc(), ValueVTs[i],
+                            SDValue(Op.getNode(), Op.getResNo() + i));
+
+  setValue(&I, DAG.getNode(ISD::MERGE_VALUES, getCurSDLoc(),
+                           DAG.getVTList(ValueVTs), Values));
 }

llvm/test/CodeGen/X86/freeze.ll

 ; X86ASM-NEXT:    retq
   %y1 = freeze {i32, i32} undef
   %v1 = extractvalue {i32, i32} %y1, 0
   %v2 = extractvalue {i32, i32} %y1, 1
   %t1 = add i32 %v1, %v2
   ret i32 %t1
 }
 
+define i32 @freeze_anonstruct2() {
+; X86ASM-LABEL: freeze_anonstruct2:
+; X86ASM:       # %bb.0:
+; X86ASM-NEXT:    movzwl %ax, %eax
+; X86ASM-NEXT:    addl %eax, %eax
+; X86ASM-NEXT:    retq
+  %y1 = freeze {i32, i16} undef
+  %v1 = extractvalue {i32, i16} %y1, 0
+  %v2 = extractvalue {i32, i16} %y1, 1
+  %z2 = zext i16 %v2 to i32
+  %t1 = add i32 %v1, %z2
+  ret i32 %t1
+}
+
 define i64 @freeze_array() {
 ; X86ASM-LABEL: freeze_array:
 ; X86ASM:       # %bb.0:
 ; X86ASM-NEXT:    addq %rax, %rax
 ; X86ASM-NEXT:    retq
   %y1 = freeze [2 x i64] undef
   %v1 = extractvalue [2 x i64] %y1, 0
   %v2 = extractvalue [2 x i64] %y1, 1
   %t1 = add i64 %v1, %v2
   ret i64 %t1
 }