<div dir="ltr">in some of the test examples, if I replace CHECK with CHECK-NEXT, the tests fail..., which suggests to me that this isn't quite true!</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 8, 2013 at 5:20 PM, Stephen Lin <span dir="ltr"><<a href="mailto:swlin@post.harvard.edu" target="_blank">swlin@post.harvard.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">As I understand it, volatile just means that the load will not be<br>
reordered with respect to other volatile loads or stores, so I don't<br>
know why it would break the test. I'll try it locally.<br>
<br>
Changing CHECK-NEXT to CHECK just means you are not longer depending<br>
on the lines being immediately after each other, but you are still<br>
depending on the loads being scheduled in the order you specified.<br>
<br>
Stephen<br>
<br>
On Mon, Jul 8, 2013 at 2:12 PM, Carter Schonwald<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:carter.schonwald@gmail.com">carter.schonwald@gmail.com</a>> wrote:<br>
> hrmm, I think those loads should not be volatile, because the semantics of<br>
> ghc means that function args are "immutable".<br>
><br>
> (tellingly, the test example fails with the volatile annotation and CHECK<br>
> instead of CHECK-NEXT, but works just fine with CHECK and no volatile)<br>
><br>
> attached is the update of the proposed patch, passes the test suite again,<br>
> i'll email the list with a separate patch for those test examples shortly<br>
><br>
><br>
> On Mon, Jul 8, 2013 at 5:05 PM, Stephen Lin <<a href="mailto:swlin@post.harvard.edu">swlin@post.harvard.edu</a>> wrote:<br>
>><br>
>> It's not that big of a deal, I think, but it's probably better to<br>
>> change them if you can. People often introduce changes that cause<br>
>> existing tests to fail just because the tests are over-specified and<br>
>> it takes time for the person making the change to figure out if the<br>
>> change actually breaks something important or not.<br>
>><br>
>> You should probably split the cleanup of the exiting tests and the new<br>
>> changes into two separate patch files (one for the changes to the<br>
>> existing tests, and one for the new stuff), since they're disjoint.<br>
>><br>
>> -Stephen<br>
>><br>
>> On Mon, Jul 8, 2013 at 1:56 PM, Carter Schonwald<br>
>> <<a href="mailto:carter.schonwald@gmail.com">carter.schonwald@gmail.com</a>> wrote:<br>
>> > Ok, will do,<br>
>> > I should remark that the current tests for vanil x86_32 and x86_64 ghc<br>
>> > calling conventions are using check-next currently, so i was copying<br>
>> > that<br>
>> > convention. Should I update the patch to change those to check too?<br>
>> ><br>
>> ><br>
>> > On Mon, Jul 8, 2013 at 4:54 PM, Stephen Lin <<a href="mailto:swlin@post.harvard.edu">swlin@post.harvard.edu</a>><br>
>> > wrote:<br>
>> >><br>
>> >> Hi Carter,<br>
>> >><br>
>> >> I'm not sure if you can really depend on loads being ordered in the<br>
>> >> order you specify them in the IR; even if they are now, they might not<br>
>> >> be in the future and it will add to the difficulty of testing changes<br>
>> >> later, so it's better not to specify things you don't actually depend<br>
>> >> upon.<br>
>> >><br>
>> >> Can you change the loads to volatile loads (just put the keyword<br>
>> >> "volatile" after the load) and change the CHECK-NEXT lines to simply<br>
>> >> CHECK? I haven't tried it yet but think that would work and be more<br>
>> >> robust to future changes.<br>
>> >><br>
>> >> Thanks,<br>
>> >> Stephen<br>
>> >><br>
>> >> On Mon, Jul 8, 2013 at 1:37 PM, Carter Tazio Schonwald<br>
>> >> <<a href="mailto:carter.schonwald@gmail.com">carter.schonwald@gmail.com</a>> wrote:<br>
>> >> > Hey All,<br>
>> >> ><br>
>> >> > currently the GHC calling convention doesnt have support for using<br>
>> >> > the<br>
>> >> > AVX<br>
>> >> > 256bit width registers. Attached is a patch that augments the GHC<br>
>> >> > x86-64<br>
>> >> > calling convention with that support when vector types of that size<br>
>> >> > are<br>
>> >> > used.<br>
>> >> ><br>
>> >> > This change has been Ok'd by the GHC HQ devs responsible for the SIMD<br>
>> >> > support recently added to ghc (as well as the principal author of the<br>
>> >> > llvm<br>
>> >> > backend for ghc ). see the ghc ticket here<br>
>> >> > <a href="http://hackage.haskell.org/trac/ghc/ticket/8033" target="_blank">http://hackage.haskell.org/trac/ghc/ticket/8033</a> for their indications<br>
>> >> > of<br>
>> >> > approval<br>
>> >> ><br>
>> >> > it'd be really great to have this patch in both the 3.4 and the<br>
>> >> > pending<br>
>> >> > 3.3<br>
>> >> > point release, because then the next GHC release could get some<br>
>> >> > additional<br>
>> >> > work to support AVX2 now rather than later. (in addition to the<br>
>> >> > current<br>
>> >> > support for 128bit simd)<br>
>> >> ><br>
>> >> > i've included the patch and an additional test case in the diff<br>
>> >> > attached<br>
>> >> > below<br>
>> >> ><br>
>> >> > thanks!<br>
>> >> > -Carter<br>
>> >> ><br>
>> >> > _______________________________________________<br>
>> >> > llvm-commits mailing list<br>
>> >> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> >> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>> >> ><br>
>> ><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div>