r185544 - Fix PR16454: Don't #include altivec.h when preprocessing assembly.
Richard Smith
richard at metafoo.co.uk
Wed Jul 3 11:34:50 PDT 2013
On Wed, Jul 3, 2013 at 11:02 AM, Bill Schmidt
<wschmidt at linux.vnet.ibm.com> wrote:
> On Wed, 2013-07-03 at 10:50 -0700, Richard Smith wrote:
>> On Wed, Jul 3, 2013 at 10:31 AM, Bill Schmidt
>> <wschmidt at linux.vnet.ibm.com> wrote:
>> > On Wed, 2013-07-03 at 10:19 -0700, Eli Friedman wrote:
>> >> On Wed, Jul 3, 2013 at 8:36 AM, Bill Schmidt
>> >> <wschmidt at linux.vnet.ibm.com> wrote:
>> >> Author: wschmidt
>> >> Date: Wed Jul 3 10:36:02 2013
>> >> New Revision: 185544
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=185544&view=rev
>> >> Log:
>> >> Fix PR16454: Don't #include altivec.h when preprocessing
>> >> assembly.
>> >>
>> >> When the -maltivec flag is present, altivec.h is auto-included
>> >> for the
>> >> compilation. This is not appropriate when the job action is
>> >> to
>> >> preprocess a file containing assembly code. So don't do that.
>> >>
>> >> I was unable to convert the test in the bug report into a
>> >> regression
>> >> test. The original symptom was exposed with:
>> >>
>> >> % touch x.S
>> >> % ./bin/clang -target powerpc64-unknown-linux-gnu -maltivec
>> >> -S -o - x.S
>> >>
>> >> I tried this test (and numerous variants) on a PPC64 system:
>> >>
>> >> ----------------------------------------------------------------------------
>> >> // RUN: touch %t
>> >> // RUN: %clang -maltivec -S %t -o - | FileCheck %s
>> >>
>> >> // Verify that assembling an empty file does not auto-include
>> >> altivec.h.
>> >>
>> >> // CHECK-NOT: static vector
>> >> ----------------------------------------------------------------------------
>> >>
>> >> However, this test passes for some reason even on a clang
>> >> built
>> >> without the fix. I'd be happy to add a test case but at this
>> >> point
>> >> I'm not able to figure one out, and I don't want to hold up
>> >> the patch
>> >> unnecessarily. Please let me know if you have ideas.
>> >>
>> >>
>> >> Umm, why are you committing a patch for an issue you can't reproduce?
>> >
>> > Sorry if I was unclear. I can reproduce the issue by hand. I can't
>> > find a way to automate the process successfully using FileCheck. I
>> > don't know why it doesn't work as a test case when it works from the
>> > command line. No doubt there is some small thing about the automatic
>> > testing process that I don't understand.
>> >
>> > I had posted this potential solution a couple of days ago in the bug,
>> > and asked for assistance with the test case. After no response, I
>> > decided to go ahead with the patch and add a test case later if I can
>> > get some help in understanding why it doesn't work.
>>
>> So... comments in bugs don't get attention from people who aren't
>> CC'd; even people subscribed to the llvmbugs@ list only see when bugs
>> are created and closed, not comments added to them. In future, please
>> send patches for which you want review or comments to cfe-commits@
>> (even if they're incomplete); you're vastly more likely to get a
>> response there. Also, as a general rule, please don't interpret a lack
>> of response as a signal to go ahead, the expectation within the
>> community is that you will ping a patch if you don't get an answer to
>> your initial mail (around once a week is typical).
>
> Sorry if this wasn't quite kosher. My intent was to request broader
> help with the test case with this patch note. I didn't want to hold up
> the original filer of the bug over this as I will be out of town for a
> number of days after today. But yes, I understand the process and will
> be more strict about it in future.
>
> Do you have a preference for the location of this test case
> (test/Driver, test/CodeGen, etc.)?
test/Driver seems appropriate. Thanks for adding the test case!
More information about the cfe-commits
mailing list