<div dir="ltr"><br><br><div class="gmail_quote">On Tue, May 5, 2015 at 10:36 AM escha <<a href="mailto:escha@apple.com">escha@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On May 4, 2015, at 3:40 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<br>
><br>
> This seems fairly reasonable, couple of nits:<br>
><br>
> a) Routine name: theoretically it should begin with a lower case letter. I know it probably doesn't match anything around it then. I don't know what we want to do about this, but I wouldn't complain much.<br>
><br>
> b) Argument names: Can you make them a little more descriptive and document them?<br>
<br>
One thing I was curious about with regards to this is we *already* have a “fast select” boolean TLI option — but it seems to represent something a bit different, that is (I think) whether we have a cmov-equivalent or selects will have to be turned into branches.<br>
<br>
Thus, we have three levels of speed:<br>
1) an actual select instruction<br>
2) we have a fast compare + cmov<br>
3) selects are slow<br>
<br>
and this patch is kind of adding 1) to the existing 2) and 3). Do we need to have an entire function to decide whether a given select is fast (might be useful on architectures that have somewhat-limited select instructions?) or should we just make a third speed level in addition to the current two?<br>
<br>
I found it kind of gross that I had to pass all five arguments (instead of just one select node) to the TLI, but unfortunately a lot of the time it seems we don’t have an actual select node already created at this point :-/<br>
<br></blockquote><div><br></div><div>*nod* Ideally I think we'd want to have a cost function, blah blah blah, maybe a tri-state is good enough for now (1, 0, -1 or an enum).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
><br>
> c) Got an in-tree user where this would be useful?<br>
<br>
I was kinda hoping someone from R600 would know, since I think I recall R600 having a select instruction? I figure it’d be useful to have some feedback from another architecture to see what they’d find useful here, since I’m not big on the idea of shoving in something solely based on an OOT arch’s needs (plus, I probably haven’t even fully thought through its possible benefits either).<br></blockquote><div><br></div><div>Yeah. Maybe poke them and the nvptx guys?</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
— escha</blockquote></div></div>