On 5/12/20 1:21 PM, Segher Boessenkool wrote:
Hi!

On Mon, May 11, 2020 at 09:56:14PM -0500, Bill Schmidt wrote:
On 5/11/20 9:48 AM, David Edelsohn wrote:
On Sun, May 10, 2020 at 9:14 AM Bill Schmidt <wschm...@linux.ibm.com>
wrote:
         * config/rs6000/altivec.md (UNSPEC_EXTRACTL): New constant.
         (UNSPEC_EXTRACTR): Likewise.
         (VEXTRACT_LR): New int iterator.
Well now the previous VSTRIR/VSTRIL patch is inconsistent.  If we're
going to use an iterator for "LR", that's fine, but it needs to be
used consistently for similar situations.  The approach for the two,
similar instructions and issues need to match.
I see your point.  I don't really like the way this was done very much,
since the attributes are tied to the unspecs for extract-{low,high}.
Simple attribute names like LR, lr, rl shouldn't be scoped so narrowly.
Yeah...  The point was to make the resulting code readable.  xx<lr> is
readable, but xx<some_long_name_lr> is not.

I don't like any of the alternatives very well, either.  I could either
(1) change the names of the int iterators in this patch to incorporate
part of the word "extract", and create similar iterators for the
vstril/vstrir patterns; or (2) remove the iterators from this patch and
just create two expansions and two insns instead of one of each.  I have
a slight preference for (2) since the longer iterator names will make
things ugly.

Do you or Segher have a preference?
Two patterns is the best idea I think.

And all of this will be less code if you can move the decision making
part to the builtin code?

OK, thanks!  The vector string isolation patch went upstream already after David's first review, but I will go back and rewrite that code in a subsequent patch.

And I will work on fixing this one and ask for re-review when it's ready.

Thanks to both of you!
Bill



Segher

Reply via email to