Another case of problems with Vec.toSeq

17 views
Skip to first unread message

Øyvind Harboe

unread,
Jan 19, 2022, 7:30:37 AMJan 19
to chisel-users
Why doesn't Vec.toSeq return Seq[T]?



import chisel3._
import chisel3.util.experimental.BoringUtils

class Foo extends Module {
  val array = IO(Input(Vec(3, Vec(3, UInt(3.W)))))
  val total = IO(Output(UInt()))

  // doesn't work  
  // total := array.reduce(_ ++ _).reduce(_ + _)
   total := array.map{s =>
     val l: Seq[UInt] = s.toSeq
                     l}.reduce(_ ++ _).reduce(_ + _)
}

println(getVerilogString(new Foo()))



Kamyar Mohajerani

unread,
Jan 19, 2022, 11:53:31 AMJan 19
to chisel...@googlegroups.com
This a due to changes made in Scala library 2.13. 

it worked with 2.12:

but toSeq is now inherited from scala.collection.immutable.IndexedSeq[A]


I think it should be possible to just change that to  scala.collection.IndexedSeq, without causing source or binary incompatibility.

-Kamyar

--
You received this message because you are subscribed to the Google Groups "chisel-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chisel-users...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/chisel-users/5b8bc84e-759b-422d-932f-1fd0877610edn%40googlegroups.com.

Øyvind Harboe

unread,
Jan 24, 2022, 10:06:37 AMJan 24
to chisel-users
I gave it a go, but it fails to compile still...

Øyvind Harboe

unread,
Jan 26, 2022, 7:11:14 AMJan 26
to chisel-users
Surprising:


  // doesn't work
  //total := array.map(_.toSeq).reduce(_ ++ _).reduce(_ + _)
  // works
  total := array.map(_.toIndexedSeq).reduce(_ ++ _).reduce(_ + _)



https://scastie.scala-lang.org/2kZzHFJzTmSaIhTt5bHKiw

Jack Koenig

unread,
Jan 27, 2022, 12:05:08 AMJan 27
to chisel...@googlegroups.com
I find this issue incredibly puzzling. I don't think https://github.com/chipsalliance/chisel3/pull/2360 is right, but I really don't understand what the issue is. It should infer the correct upcast to `IndexedSeq`...

Anyway, the usual technique of adding type parameters where they are usually inferred resolves this for me, ie.

total := array.reduce[Seq[UInt]](_ ++ _).reduce(_ + _)

I have no idea why the compiler is not figuring this out for us though.

I'm assuming your actual use case is more complex, but I will note that `.reduce(_ ++ _)` on any Seq is better as just a flatten, eg.

total := array.flatten.reduce(_ + _)

--
You received this message because you are subscribed to the Google Groups "chisel-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chisel-users...@googlegroups.com.

Øyvind Harboe

unread,
Jan 27, 2022, 6:41:50 AMJan 27
to chisel-users
Thanks!

I have closed the PR. Before the issues with the Scala compiler/type checking is fully understood, there's no point in trying to change Chisel :-)

On Thursday, January 27, 2022 at 6:05:08 AM UTC+1 Jack Koenig wrote:
I find this issue incredibly puzzling. I don't think https://github.com/chipsalliance/chisel3/pull/2360 is right, but I really don't understand what the issue is. It should infer the correct upcast to `IndexedSeq`...

Anyway, the usual technique of adding type parameters where they are usually inferred resolves this for me, ie.

total := array.reduce[Seq[UInt]](_ ++ _).reduce(_ + _)

I have no idea why the compiler is not figuring this out for us though.

I'm assuming your actual use case is more complex,

Yes. This was just the smallest example I could make of the problem I was seeing.
Reply all
Reply to author
Forward
0 new messages