Foundations for the optimizer & cleanup#25187
Foundations for the optimizer & cleanup#25187SolalPirelli wants to merge 8 commits intoscala:mainfrom
Conversation
| import dotty.tools.io.FileWriters.{BufferingReporter, Report} | ||
|
|
||
|
|
||
| sealed trait BackendReporting { |
There was a problem hiding this comment.
Moved from PostProcessorFrontendAccess + added the code that was previously in the io package but really belongs here. No logic changes.
| import frontendAccess.{compilerSettings} | ||
| import bTypes.* | ||
| import coreBTypes.jliLambdaMetaFactoryAltMetafactoryHandle | ||
| class BackendUtils(val ppa: PostProcessorFrontendAccess, val ts: CoreBTypes)(using Context) { |
There was a problem hiding this comment.
This is the pattern that repeats in this PR: make dependencies explicit, and move some members as a result so classes can be layered instead of one big cyclic object graph.
|
|
||
| import Primitives.TestOp | ||
|
|
||
| private object DesugaredSelect { |
There was a problem hiding this comment.
Only used in this class so moved from DottyBackendInterface. No logic changes.
| genLoad(t, generatedType) | ||
| if (!sym.is(Package)) { | ||
| if (sym.is(Module)) genLoadModule(sym) | ||
| else locals.load(sym) |
There was a problem hiding this comment.
This was dead code, the Some branch was unreachable because we'd have matched a previous arm of the outer match.
| app match { | ||
| case Apply(_, args) if app.symbol eq defn.newArrayMethod => | ||
| val List(elemClaz, Literal(c: Constant), ArrayValue(_, dims)) = args: @unchecked | ||
| val List(elemClaz, Literal(c: Constant), av: tpd.JavaSeqLiteral) = args: @unchecked |
There was a problem hiding this comment.
ArrayValue was a complex extractor that wasn't actually needed in all but one case, so I inlined that case and simplified the rest.
| end emitLocalVarScopes | ||
|
|
||
| def adapt(from: BType, to: BType): Unit = { | ||
| if (!from.conformsTo(to)) { |
There was a problem hiding this comment.
Porting a change made in scala2
|
|
||
| private def emitArgument(av: AnnotationVisitor, | ||
| name: String, | ||
| arg: Tree, bcodeStore: BCodeHelpers)(innerClasesStore: bcodeStore.BCInnerClassGen): Unit = { |
There was a problem hiding this comment.
No idea why this (instance!) method was taking itself as an extra argument, but the code works fine without it.
| private def compilingArray(using Context) = | ||
| ctx.compilationUnit.source.file.name == "Array.scala" | ||
|
|
||
| private val primitiveCompilationUnits = Set( |
There was a problem hiding this comment.
This and below were moved from where they were declared because this class was their only use. We can/should centralize stuff like this as I wrote in the PR description, but that's for after adding the optimizer when we can have a full view of what utilities are needed...
| .addFlagIf(sym.hasAnnotation(TransientAttr), ACC_TRANSIENT) | ||
| .addFlagIf(sym.hasAnnotation(VolatileAttr), ACC_VOLATILE) | ||
| .addFlagIf(!sym.is(Mutable), ACC_FINAL) | ||
| } |
There was a problem hiding this comment.
Used to be in BTypesFromSymbols for some reason.
|
|
||
| object BCodeUtils { | ||
|
|
||
| /** |
There was a problem hiding this comment.
Used to be in BTypesFromSymbols but it's stateless and the optimizer needs it in other places.
| * This representation is immutable and independent of the compiler data structures, hence it can | ||
| * be queried by concurrent threads. | ||
| */ | ||
| abstract class BTypes { self => |
There was a problem hiding this comment.
GitHub makes a mess of this diff because of the indentation changes but basically the class BTypes no longer exists, BType and its subtypes are top-level now.
| private lazy val _jliMethodHandleRef: Lazy[ClassBType] = perRunLazy(classBTypeFromSymbol(defn.MethodHandleClass)) | ||
| def ObjectRef : ClassBType | ||
| def StringRef : ClassBType | ||
| def PredefRef : ClassBType |
There was a problem hiding this comment.
Added stuff the optimizer needs. I could revert here but it's very self-contained (decl here + impl in CoreBTypesFromSymbols)
| def typeOfArrayOp: Map[Int, BType] | ||
| } | ||
|
|
||
| abstract class CoreBTypesFromSymbols[I <: DottyBackendInterface] extends CoreBTypes { |
There was a problem hiding this comment.
Moved to its own file, no logic changes
| private val entryPoints = new mutable.HashSet[String]() | ||
| def registerEntryPoint(s: String): Unit = entryPoints += s | ||
|
|
||
| private var _backendInterface: DottyBackendInterface = uninitialized |
There was a problem hiding this comment.
Messy diff but it's just moving the vars/defs around so they're in layering order
| override def getPrimitive(app: Apply, tpe: Type)(using Context): Int = | ||
| jsPrimitives.getOrElse(app.fun.symbol, super.getPrimitive(app, tpe)) | ||
| override def getPrimitive(app: Apply, tpe: Type): Int = | ||
| jsPrimitives.getOrElse(app.fun.symbol(using ictx), super.getPrimitive(app, tpe)) |
There was a problem hiding this comment.
Since I refactored this for consistency with other methods in DottyPrimitives I also had to refactor it here. Whether the JS backend should have a dependency on a class from the JVM backend is a question for another day.
| import java.util.ConcurrentModificationException | ||
|
|
||
| /** !!!Copied from `dotty.tools.backend.jvm.ClassfileWriters` but no `PostProcessorFrontendAccess` needed. | ||
| * this should probably be changed to wrap that class instead. |
There was a problem hiding this comment.
I'm pretty sure we don't want the io package wrapping the backend/jvm package. It should be the other way around.
| res | ||
| } | ||
|
|
||
| private def firstCommonSuffix(as: List[ClassBType], bs: List[ClassBType]): ClassBType = { |
There was a problem hiding this comment.
One important change here, this method has specific expectations for the order and contents of its arguments, but one scala2 commit that touches the creation of said arguments had only been half backported so it could fail in some cases... thus I backported the other half of the commit, which is a change in this method.
| final def isNonVoidPrimitiveType = isPrimitive && this != UNIT | ||
|
|
||
| final def isNullType = this == srNullRef | ||
| final def isNothingType = this == srNothingRef |
There was a problem hiding this comment.
Since these props depended on the current null/nothing/... refs, I made them abstract and ClassBType now takes a CoreBTypes to compare itself to them. I still think we're translating data back and forth between formats (symbols, classes, btypes) in sub-optimal ways but that's for later.
Prerequisite for #25165, extracting the stuff that touches the backend without actually being the optimizer, and cleaning up the overall architecture so it can be layered (this is especially important when adding back the optimizer so it's understandable...)
I realize this PR is a little larger than is reasonable, sorry about that. Part of the point of the refactoring was that by following the code flow and simplifying it I got an understanding of how the JVM backend works...
Main changes:
BTypeand its implementations are top-level now, no longer members of a module-like classCoreBTypesis passed explicitly where neededBTypesFromSymbolsintegrated intoCoreBTypesFromSymbols(since its parentBTypesno longer exists)CoreBTypesFromSymbolsmoved to its own fileDottyBackendInterfaceis anobjectonly, the stateful stuff it did has been inlined where it was neededbTypes/int/postProcessor. This makes dependencies explicit, and allowed me to remove some unneeded cycles in the object graph (especially in the next PR which is the optimizer)BackendReportingmoved to its own fileiopackage no longer has a dependency on thebackend/jvmpackage (!!!)There's a lot of extra cleanup that could be done, and that I'd like to do once the optimizer is in, since we have many "utility" classes that could be either a single class or split in a way that makes sense (e.g.,
AsmUtilsis currently just about... logging). From what I understand some of this dates back to the porting of the JVM backend to Dotty, which introduced "temporary" interfaces to the rest of the compiler.And more generally the overall state of the backend is spread across many classes that often touch what should be each other's internals (this is especially pronounced with the call graph, which will be added with the optimizer), and it's not easy to follow the data flow.
But that's for later.