Page MenuHomeFreeBSD
Differential D25154

skein_block_asm.s: use #ifdef not .ifdef, for Clang IAS
ClosedPublic
Actions

Authored by emaste on Jun 5 2020, 5:44 PM.
Tags
None
Referenced Files
F167916617: D25154.diff
Tue, Aug 25, 10:19 AM
F167882448: D25154.id.diff
Tue, Aug 25, 5:50 AM
Unknown Object (File)
Fri, Aug 21, 4:17 PM
Unknown Object (File)
Fri, Aug 21, 4:00 PM
Unknown Object (File)
Thu, Aug 20, 8:53 PM
Unknown Object (File)
Wed, Aug 19, 4:53 PM
Unknown Object (File)
Sun, Aug 9, 9:55 PM
Unknown Object (File)
Sun, Aug 9, 3:08 PM
View All 96 Files
Subscribers
imp

Details

Reviewers
arichardson
allanjude
cem
jmg
Commits
rS361843: skein_block_asm.S: use #ifdef not .ifdef, for Clang IAS
Summary

Clang IAS does not support the --defsym argument, and
.ifndef SKEIN_USE_ASM
gets turned into
.ifndef 1792
by the preprocessor, which results in an error

error: expected identifier after '.ifdef'
    .ifndef 1792
            ^

GNU as works with both #ifdef and .ifdef.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste requested review of this revision.Jun 5 2020, 5:44 PM
emaste created this revision.
cem accepted this revision.Jun 5 2020, 5:56 PM
This revision is now accepted and ready to land.Jun 5 2020, 5:56 PM
Closed by commit rS361843: skein_block_asm.S: use #ifdef not .ifdef, for Clang IAS (authored by emaste). · Explain WhyJun 5 2020, 6:37 PM
This revision was automatically updated to reflect the committed changes.
emaste added a commit: rS361843: skein_block_asm.S: use #ifdef not .ifdef, for Clang IAS.
Herald added a reviewer: jmg. · View Herald TranscriptJun 5 2020, 6:37 PM
Herald added a subscriber: imp. · View Herald Transcript
arichardson added a comment.Jun 5 2020, 7:37 PM

The other option would have been to change the build system to pass -Wa,-defsym, SKEIN_USE_ASM=XXX (that's what I did for my godbolt testing.
But either solution seems fine.

emaste added a comment.Jun 5 2020, 7:42 PM
In D25154#554268, @arichardson wrote:

The other option would have been to change the build system to pass -Wa,-defsym, SKEIN_USE_ASM=XXX (that's what I did for my godbolt testing.
But either solution seems fine.

Oh hmm, I tried that and had an error, but now I see I tried --defsym with two dashes.

arichardson added a comment.Jun 5 2020, 7:44 PM
In D25154#554275, @emaste wrote:
In D25154#554268, @arichardson wrote:

The other option would have been to change the build system to pass -Wa,-defsym, SKEIN_USE_ASM=XXX (that's what I did for my godbolt testing.
But either solution seems fine.

Oh hmm, I tried that and had an error, but now I see I tried --defsym with two dashes.

I haven't checked if there's an open LLVM bug, but clang should probably accept the -- version for compatibility.

Revision Contents
Changeset List

PathSize
head/
sys/
crypto/
skein/
amd64/
13 lines

Diff 72732

View Options

head/sys/crypto/skein/amd64/skein_block_asm.s

Loading...