Skip to content

SI-10037 ASR/LSR switched in ICodeReader #5518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 11, 2016
Merged

SI-10037 ASR/LSR switched in ICodeReader #5518

merged 1 commit into from
Nov 11, 2016

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Nov 10, 2016 •
edited by SethTisue
Loading

Noticed when inlining from a class file.

JIRA: https://issues.scala-lang.org/browse/SI-10037
@scala-jenkins scala-jenkins added this to the 2.11.9 milestone Nov 10, 2016
lrytz
lrytz requested changes Nov 10, 2016
View reviewed changes
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Could you mark the commit [nomerge]?
src/compiler/scala/tools/nsc/backend/icode/Primitives.scala
case ASR => "ASR"
case LSR => "LSR"
case _ => throw new RuntimeException("ShitOp unknown case")
case _ => throw new RuntimeException("ShiftOp unknown case")
Copy link
Member

@lrytz lrytz Nov 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😳
@retronym
Copy link
Member

retronym commented Nov 10, 2016

/rebuild
@SethTisue
Copy link
Member

SethTisue commented Nov 10, 2016 

!! 697 - run/t10037 [output differs]
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 10, 2016

Thanks @SethTisue . I was a bit casual b/c home with flu; I forgot the flags. Now I see that this test isn't inlining. If anyone reads this & knows why offhand, please let me know. Maybe I'll try a ReplTest in case the class loaders align.

scala.reflect.internal.MissingRequirementError: object X in compiler mirror not found. at scala.reflect.internal.MissingRequirementError$.signal(MissingRequirementError.scala:17) at scala.reflect.internal.MissingRequirementError$.notFound(MissingRequirementError.scala:18) at scala.reflect.internal.Mirrors$RootsBase$$anonfun$getModuleOrClass$3.apply(Mirrors.scala:54) at scala.reflect.internal.Mirrors$RootsBase$$anonfun$getModuleOrClass$3.apply(Mirrors.scala:54) at scala.reflect.internal.Symbols$Symbol.orElse(Symbols.scala:2536) at scala.reflect.internal.Mirrors$RootsBase.getModuleOrClass(Mirrors.scala:53) at scala.reflect.internal.Mirrors$RootsBase.getModuleOrClass(Mirrors.scala:66) at scala.reflect.internal.Mirrors$RootsBase.getModuleByName(Mirrors.scala:134) at scala.tools.nsc.symtab.classfile.ClassfileParser$ConstantPool.getClassSymbol(ClassfileParser.scala:230) at scala.tools.nsc.symtab.classfile.ICodeReader.parseClass(ICodeReader.scala:146) at scala.tools.nsc.symtab.classfile.ClassfileParser$$anonfun$parse$1.apply$mcV$sp(ClassfileParser.scala:150) at scala.tools.nsc.symtab.classfile.ClassfileParser$$anonfun$parse$1.apply(ClassfileParser.scala:139) at scala.tools.nsc.symtab.classfile.ClassfileParser$$anonfun$parse$1.apply(ClassfileParser.scala:139) at scala.tools.nsc.symtab.classfile.ClassfileParser.pushBusy(ClassfileParser.scala:125) at scala.tools.nsc.symtab.classfile.ClassfileParser.parse(ClassfileParser.scala:139) at scala.tools.nsc.symtab.classfile.ICodeReader.readClass(ICodeReader.scala:134) at scala.tools.nsc.backend.icode.Repository$class.load(Repository.scala:32) at scala.tools.nsc.backend.icode.ICodes.load(ICodes.scala:19) at scala.tools.nsc.backend.opt.Inliners$Inliner.scala$tools$nsc$backend$opt$Inliners$Inliner$$analyzeInc$1(Inliners.scala:407)
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 10, 2016

ReplTest wasn't any happier. I'll experiment later. Here's the manual test, FTR:

$ scala211 -optimise -Yinline-warnings Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_101). Type in expressions for evaluation. Or try :help. scala> shifty.X.f(Int.MinValue) res0: Int = 1073741824 scala> shifty.X.g(Int.MinValue) res1: Int = -1073741824 scala> :quit $ skala -optimise -Yinline-warnings Welcome to Scala 2.11.9-20161108-113317-74ed575a0f (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_101). Type in expressions for evaluation. Or try :help. scala> shifty.X.f(Int.MinValue) res0: Int = -1073741824 scala> shifty.X.g(Int.MinValue) res1: Int = 1073741824
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 10, 2016

Maybe I'll just delete the test.
@retronym
Copy link
Member

retronym commented Nov 11, 2016

The old optimizer baulks at the sight of the empty package. Bearing this in mind, we can revive the test case.
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 11, 2016

Aha! Thanks for the hint. I got rid of packages because partest doesn't descend looking for a Test IIRC.
@som-snytt
[nomerge] SI-10037 ASR/LSR switched in ICodeReader
1dfdb9c 
Noticed when inlining from a class file. The test doesn't work because inlining fails with bytecode unavailable due to: ``` scala.reflect.internal.MissingRequirementError: object X in compiler mirror not found. ```
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 11, 2016

It's such a great feeling when your PR reaches the Graphviz error output.
@retronym retronym merged commit 35f8908 into scala:2.11.x Nov 11, 2016
@som-snytt som-snytt deleted the issue/10037-2.11 branch November 11, 2016 07:44
@scabug scabug mentioned this pull request Apr 7, 2017
Closed
@SethTisue SethTisue modified the milestones: 2.11.11, 2.11.9 May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants

@som-snytt @retronym @SethTisue @lrytz @scala-jenkins