[PATCH] gnu: Make vice tunable.

  • Open
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • raingloom
Owner
unassigned
Submitted by
raingloom
Severity
normal
R
R
raingloom wrote on 13 Sep 2023 05:48
(address . guix-patches@gnu.org)
4a6e24ae29f8adca5aa80334f9e07955d15175a6.1694576934.git.raingloom@riseup.net
From: Csepp <raingloom@riseup.net>

* gnu/packages/emulators.scm (vice)[properties]: Set tunable? to #t.
---
This fixes the issue with unsupported AVX instructions.

gnu/packages/emulators.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (16 lines)
diff --git a/gnu/packages/emulators.scm b/gnu/packages/emulators.scm
index 1d50c9ef01..dd1e3e877f 100644
--- a/gnu/packages/emulators.scm
+++ b/gnu/packages/emulators.scm
@@ -118,6 +118,7 @@ (define-public vice
(package
(name "vice")
(version "3.7.1")
+ (properties '((tunable? . #t)))
(source
(origin
(method url-fetch)

base-commit: 07d43c66d5c11fef61f9846fefb97fa18e4764f1
--
2.41.0
L
L
Ludovic Courtès wrote on 14 Sep 2023 16:46
(name . raingloom)(address . raingloom@riseup.net)
87o7i4ak43.fsf@gnu.org
Hi,

raingloom <raingloom@riseup.net> skribis:

Toggle quote (6 lines)
> From: Csepp <raingloom@riseup.net>
>
> * gnu/packages/emulators.scm (vice)[properties]: Set tunable? to #t.
> ---
> This fixes the issue with unsupported AVX instructions.

Could you clarify what this means, ideally as a comment above the
property?

Thanks,
Ludo’.
R
R
raingloom wrote on 14 Sep 2023 21:47
[PATCH v2] gnu: Make vice tunable.
(address . 65903@debbugs.gnu.org)
6fc797b4ee3c49cb7fb730b4ba1664a733ebf0df.1694720820.git.raingloom@riseup.net
From: Csepp <raingloom@riseup.net>

* gnu/packages/emulators.scm (vice)[properties]: Set tunable? to #t.
---
gnu/packages/emulators.scm | 2 ++
1 file changed, 2 insertions(+)

Toggle diff (17 lines)
diff --git a/gnu/packages/emulators.scm b/gnu/packages/emulators.scm
index 1d50c9ef01..0fb4a5853b 100644
--- a/gnu/packages/emulators.scm
+++ b/gnu/packages/emulators.scm
@@ -118,6 +118,8 @@ (define-public vice
(package
(name "vice")
(version "3.7.1")
+ ;; without this it would use AVX even when it's not suported:
+ (properties '((tunable? . #t)))
(source
(origin
(method url-fetch)

base-commit: 07d43c66d5c11fef61f9846fefb97fa18e4764f1
--
2.41.0
L
L
Ludovic Courtès wrote on 17 Sep 2023 12:05
(name . raingloom)(address . raingloom@riseup.net)(address . 65903@debbugs.gnu.org)
871qex15ex.fsf@gnu.org
Hi,

raingloom <raingloom@riseup.net> skribis:

Toggle quote (17 lines)
> From: Csepp <raingloom@riseup.net>
>
> * gnu/packages/emulators.scm (vice)[properties]: Set tunable? to #t.
> ---
> gnu/packages/emulators.scm | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gnu/packages/emulators.scm b/gnu/packages/emulators.scm
> index 1d50c9ef01..0fb4a5853b 100644
> --- a/gnu/packages/emulators.scm
> +++ b/gnu/packages/emulators.scm
> @@ -118,6 +118,8 @@ (define-public vice
> (package
> (name "vice")
> (version "3.7.1")
> + ;; without this it would use AVX even when it's not suported:

Oh I see. It’s a problem that ‘tunable?’ in itself doesn’t solve.

The fix would be twofold: (1) remove ‘-march’ and similar compiler
arguments that cause it to generate code that assumes AVX availability,
and (2) add the ‘tune?’ property for those who want a fine-tuned binary.

Could you look into it?

Thanks,
Ludo’.
C
C
Csepp wrote on 18 Sep 2023 14:17
(name . Ludovic Courtès)(address . ludo@gnu.org)
cuc5y47ptao.fsf@riseup.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (32 lines)
> Hi,
>
> raingloom <raingloom@riseup.net> skribis:
>
>> From: Csepp <raingloom@riseup.net>
>>
>> * gnu/packages/emulators.scm (vice)[properties]: Set tunable? to #t.
>> ---
>> gnu/packages/emulators.scm | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/gnu/packages/emulators.scm b/gnu/packages/emulators.scm
>> index 1d50c9ef01..0fb4a5853b 100644
>> --- a/gnu/packages/emulators.scm
>> +++ b/gnu/packages/emulators.scm
>> @@ -118,6 +118,8 @@ (define-public vice
>> (package
>> (name "vice")
>> (version "3.7.1")
>> + ;; without this it would use AVX even when it's not suported:
>
> Oh I see. It’s a problem that ‘tunable?’ in itself doesn’t solve.
>
> The fix would be twofold: (1) remove ‘-march’ and similar compiler
> arguments that cause it to generate code that assumes AVX availability,
> and (2) add the ‘tune?’ property for those who want a fine-tuned binary.
>
> Could you look into it?
>
> Thanks,
> Ludo’.

Umm, it definitely solved it on my machine, which doesn't have AVX.
If that's not a "proper" fix, then sure, I can try looking into it more.
But it seems to me like `tunable?` is already working as intended.
L
L
Ludovic Courtès wrote on 19 Sep 2023 11:45
(name . Csepp)(address . raingloom@riseup.net)(address . 65903@debbugs.gnu.org)
87il86ebsn.fsf@gnu.org
Hi,

Csepp <raingloom@riseup.net> skribis:

Toggle quote (2 lines)
> Ludovic Courtès <ludo@gnu.org> writes:

[...]

Toggle quote (13 lines)
>> The fix would be twofold: (1) remove ‘-march’ and similar compiler
>> arguments that cause it to generate code that assumes AVX availability,
>> and (2) add the ‘tune?’ property for those who want a fine-tuned binary.
>>
>> Could you look into it?
>>
>> Thanks,
>> Ludo’.
>
> Umm, it definitely solved it on my machine, which doesn't have AVX.
> If that's not a "proper" fix, then sure, I can try looking into it more.
> But it seems to me like `tunable?` is already working as intended.

What I meant is that someone installing ‘vice’ without ‘--tune’ will
still get a binary that assumes AVXsomething, which may or may not work
on their machine.

The default binary (without ‘--tune’) should target baseline x86_64.

That’s why we need both fixes I mentioned above.

HTH!

Ludo’.
L
L
Ludovic Courtès wrote on 8 Oct 2023 23:36
control message for bug #65903
(address . control@debbugs.gnu.org)
87pm1ox0d4.fsf@gnu.org
tags 65903 + moreinfo
quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 65903@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 65903
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch