[GlobalISel] Fix offsets to valueIsSplit

[aemerson]

Summary

This fix is for IRtranslator. Offsets passed to valueIsSplit() are stored by appending which ends up continuously appending offsets which were already resolved for the same type.

Here is a test case for our target were the two structs are passed in 4x 64b registers.

define i64 @param_two_struct([2 x i64] %t.coerce, [2 x i64] %s.coerce) {
entry:

%t.coerce.fca.0.extract = extractvalue [2 x i64] %t.coerce, 0
%s.coerce.fca.1.extract = extractvalue [2 x i64] %s.coerce, 1
%add = add nsw i64 %s.coerce.fca.1.extract, %t.coerce.fca.0.extract
ret i64 %add

[aemerson]

Could you upload with more context please, and a test case?

Thanks,
Amara

[aemerson]

Thinking about this more, I can't think of any reason why we'd ever want to add offsets to an existing offset list. It would probably be better if valueIsSpit cleared the vector (and the doc comment made this clear).

[aemerson]

I'll take over and fix this in the way I suggested. Thanks for reporting this!

[asmith]

Been out of town. Thanks for fixing this!

llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h

private:
   /// Combines all component registers of \p V into a single scalar with size
   /// "max(Offsets) + last size".
   unsigned packRegs(const Value &V, MachineIRBuilder &MIRBuilder);
 
   void unpackRegs(const Value &V, unsigned Src, MachineIRBuilder &MIRBuilder);
 
   /// Returns true if the value should be split into multiple LLTs.
   /// If \p Offsets is given then the split type's offsets will be stored in it.
+  /// If \p Offsets is not empty it will be cleared first.
   bool valueIsSplit(const Value &V,
                     SmallVectorImpl<uint64_t> *Offsets = nullptr);
 
   /// Translate call instruction.
   /// \pre \p U is a call instruction.
   bool translateCall(const User &U, MachineIRBuilder &MIRBuilder);
 
   bool translateInvoke(const User &U, MachineIRBuilder &MIRBuilder);

llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp

for (unsigned i = 0; i < PI->getNumIncomingValues(); ++i) {
       }
     }
   }
 }
 
 bool IRTranslator::valueIsSplit(const Value &V,
                                 SmallVectorImpl<uint64_t> *Offsets) {
   SmallVector<LLT, 4> SplitTys;
+  if (Offsets && !Offsets->empty())
+    Offsets->clear();
   computeValueLLTs(*DL, *V.getType(), SplitTys, Offsets);
   return SplitTys.size() > 1;
 }
 
 bool IRTranslator::translate(const Instruction &Inst) {
   CurBuilder.setDebugLoc(Inst.getDebugLoc());
   switch(Inst.getOpcode()) {
 #define HANDLE_INST(NUM, OPCODE, CLASS) \

llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-duplicate-types-param.ll

+; RUN: llc -O0 -o - -verify-machineinstrs %s | FileCheck %s
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check we don't crash due to encountering the same struct param type twice.
+; CHECK-LABEL: param_two_struct
+; CHECK: add
+; CHECK: ret
+define i64 @param_two_struct([2 x i64] %t.coerce, [2 x i64] %s.coerce) {
+entry:
+  %t.coerce.fca.0.extract = extractvalue [2 x i64] %t.coerce, 0
+  %s.coerce.fca.1.extract = extractvalue [2 x i64] %s.coerce, 1
+  %add = add nsw i64 %s.coerce.fca.1.extract, %t.coerce.fca.0.extract
+  ret i64 %add
+}