I have run check_GNU_style.sh on my patch. The patch is submitted. Thank you for your comments and help on this patch!
thanks, Cong On Wed, Oct 30, 2013 at 11:13 AM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Wed, Oct 30, 2013 at 7:01 PM, Cong Hou <co...@google.com> wrote: > >>>> I found my problem: I put DONE outside of if not inside. You are >>>> right. I have updated my patch. >>> >>> OK, great that we put things in order ;) >>> >>> Does this patch need some extra middle-end functionality? I was not >>> able to vectorize char and short part of your patch. >> >> >> In the original patch, I converted abs() on short and char values to >> their own types by removing type casts. That is, originally char_val1 >> = abs(char_val2) will be converted to char_val1 = (char) abs((int) >> char_val2) in the frontend, and I would like to convert it back to >> char_val1 = abs(char_val2). But after several discussions, it seems >> this conversion has some problems such as overflow converns, and I >> thereby removed that part. >> >> Now you should still be able to vectorize abs(char) and abs(short) but >> with packing and unpacking. Later I will consider to write pattern >> recognizer for abs(char) and abs(short) and then the expand on >> abs(char)/abs(short) in this patch will be used during vectorization. > > OK, this seems reasonable. We already have "unused" SSSE3 8/16 bit abs > pattern, so I think we can commit SSE2 expanders, even if they will be > unused for now. The proposed recognizer will benefit SSE2 as well as > existing SSSE3 patterns. > >>> Regarding the testcase - please put it to gcc.target/i386/ directory. >>> There is nothing generic in the test, as confirmed by target-dependent >>> scan test. You will find plenty of examples in the mentioned >>> directory. I'd suggest to split the testcase in three files, and to >>> simplify it to something like the testcase with global variables I >>> used earlier. >> >> >> I have done it. The test case is split into three for s8/s16/s32 in >> gcc.target/i386. > > OK. > > The patch is OK for mainline, but please check formatting and > whitespace before the patch is committed. > > Thanks, > Uros.
