[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
?