[PATCH] D29069: [MSP430] Add SRet support to MSP430 target

[Anton Korobeynikov via Phabricator via llvm-commits]

asl added inline comments.


================
Comment at: lib/Target/MSP430/MSP430ISelLowering.cpp:248
                               SmallVectorImpl<unsigned> &Out) {
   unsigned CurrentArgIndex = ~0U;
   for (unsigned i = 0, e = Args.size(); i != e; i++) {
----------------
I'm a bit confused by logic here. Is there any reason why we shouldn't simply count from 0 here? Could also switch to range-for while here?


================
Comment at: lib/Target/MSP430/MSP430ISelLowering.cpp:252
+        CurrentArgIndex == Args[i].OrigArgIndex) {
       Out.back()++;
     } else {
----------------
Minor nit: I'd prefer += 1 here.


================
Comment at: lib/Target/MSP430/MSP430ISelLowering.cpp:324
+      State.addLoc(CCValAssign::getReg(ValNo++, ArgVT, Reg, LocVT, LocInfo));
+      RegsLeft--;
+      
----------------
Minor nit: I'd prefer -= 1 here


================
Comment at: lib/Target/MSP430/MSP430ISelLowering.cpp:328
+      CC_MSP430_AssignStack(ValNo++, ArgVT, LocVT, LocInfo, ArgFlags, State);
+    }
+    else if (Parts <= RegsLeft) {
----------------
Please follow LLVM's coding style : no newline before "else if" / "else"


================
Comment at: lib/Target/MSP430/MSP430ISelLowering.cpp:570
+    
+    if (!Reg) {
+      llvm_unreachable("sret virtual register not created in entry block");
----------------
No braces


https://reviews.llvm.org/D29069