[PATCH] [llgo] Elide alloca for unused received values in select

Peter Collingbourne peter at pcc.me.uk
Tue Dec 30 15:41:18 PST 2014


LGTM. Comment is mostly nitpicking, this is fine as is.


================
Comment at: irgen/channels.go:71
@@ -69,4 +70,3 @@
 
-func (fr *frame) chanSelect(states []selectState, blocking bool) (index, recvOk *govalue, recvElems []*govalue) {
-	n := uint64(len(states))
-	if !blocking {
+func chanSelect(fr *frame, sel *ssa.Select) (index, recvOk *govalue, recvElems []*govalue) {
+	n := uint64(len(sel.States))
----------------
axw wrote:
> pcc wrote:
> > Was there any particular reason to make this a top-level function rather than a method?
> It feels a little odd for frame to own methods; the frame is the subject here, so it feels more natural to have it as a top-level function operating on a frame. I'll change it back for now, for consistency's sake.
Right, if we decide this is worth changing we should do it globally rather than be inconsistent.

================
Comment at: irgen/channels.go:136
@@ +135,3 @@
+		}
+		return len(*extract.Referrers()) > 0
+	}
----------------
axw wrote:
> pcc wrote:
> > There could in principle be multiple `extract` referrers. With this code you end up checking only the first one.
> The loop and index check should take care of multiple extract instructions. I don't think there would ever be multiple extract referrers with the same index, would there?
Yes, I meant multiple `extract` referrers with the same index. I agree that we probably won't ever see this, but it wouldn't hurt to check for them.

http://reviews.llvm.org/D6785

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list