Skip to content

Commit c36e8e6

Browse files
DSouzaMDSouzaM
committed
Fix unbound local checks, remove unused/redundant operations
1 parent b6e0fda commit c36e8e6

File tree

2 files changed

+85
-156
lines changed

2 files changed

+85
-156
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java

+43-38
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.oracle.graal.python.builtins.objects.ellipsis.PEllipsis;
2020
import com.oracle.graal.python.builtins.objects.function.PArguments;
2121
import com.oracle.graal.python.builtins.objects.function.PKeyword;
22+
import com.oracle.graal.python.builtins.objects.type.TypeFlags;
2223
import com.oracle.graal.python.compiler.CompilationScope;
2324
import com.oracle.graal.python.compiler.Compiler;
2425
import com.oracle.graal.python.compiler.Compiler.ConstantCollection;
@@ -845,7 +846,11 @@ private BytecodeDSLCompilerResult buildComprehensionCodeUnit(SSTNode node, Compr
845846
@Override
846847
public BytecodeDSLCompilerResult visit(ExprTy.ListComp node) {
847848
return buildComprehensionCodeUnit(node, node.generators, "<listcomp>",
848-
(statementCompiler) -> statementCompiler.b.emitMakeEmptyList(),
849+
(statementCompiler) -> {
850+
statementCompiler.b.beginMakeList();
851+
statementCompiler.b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
852+
statementCompiler.b.endMakeList();
853+
},
849854
(statementCompiler, collection) -> {
850855
statementCompiler.b.beginListAppend();
851856
statementCompiler.b.emitLoadLocal(collection);
@@ -857,7 +862,10 @@ public BytecodeDSLCompilerResult visit(ExprTy.ListComp node) {
857862
@Override
858863
public BytecodeDSLCompilerResult visit(ExprTy.DictComp node) {
859864
return buildComprehensionCodeUnit(node, node.generators, "<dictcomp>",
860-
(statementCompiler) -> statementCompiler.b.emitMakeEmptyDict(),
865+
(statementCompiler) -> {
866+
statementCompiler.b.beginMakeDict(0);
867+
statementCompiler.b.endMakeDict();
868+
},
861869
(statementCompiler, collection) -> {
862870
statementCompiler.b.beginSetDictItem();
863871
statementCompiler.b.emitLoadLocal(collection);
@@ -870,7 +878,11 @@ public BytecodeDSLCompilerResult visit(ExprTy.DictComp node) {
870878
@Override
871879
public BytecodeDSLCompilerResult visit(ExprTy.SetComp node) {
872880
return buildComprehensionCodeUnit(node, node.generators, "<setcomp>",
873-
(statementCompiler) -> statementCompiler.b.emitMakeEmptySet(),
881+
(statementCompiler) -> {
882+
statementCompiler.b.beginMakeSet();
883+
statementCompiler.b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
884+
statementCompiler.b.endMakeSet();
885+
},
874886
(statementCompiler, collection) -> {
875887
statementCompiler.b.beginSetAdd();
876888
statementCompiler.b.emitLoadLocal(collection);
@@ -968,21 +980,14 @@ private void emitNameFastOperation(String mangled, NameOperation op, Builder b)
968980
BytecodeLocal local = locals.get(mangled);
969981
switch (op) {
970982
case Read:
971-
b.beginCheckUnboundLocal(varnames.get(mangled));
972-
b.emitLoadLocal(local);
973-
b.endCheckUnboundLocal();
974-
break;
975-
case Delete:
976983
b.beginBlock();
977-
b.beginCheckUnboundLocal(varnames.get(mangled));
984+
b.emitCheckUnboundLocal(local, varnames.get(mangled));
978985
b.emitLoadLocal(local);
979-
b.endCheckUnboundLocal();
980-
981-
b.beginStoreLocal(local);
982-
b.emitLoadNull();
983-
b.endStoreLocal();
984986
b.endBlock();
985987
break;
988+
case Delete:
989+
b.emitDeleteLocal(local, varnames.get(mangled));
990+
break;
986991
case BeginWrite:
987992
if (local == null) {
988993
throw new NullPointerException("local " + mangled + " not defined");
@@ -1142,7 +1147,7 @@ public void setUpFrame(ArgumentsTy args, Builder b) {
11421147
List<BytecodeLocal> toClear = new ArrayList<>();
11431148

11441149
b.beginStoreRange(cellVariableLocals);
1145-
b.beginMakeVariadic();
1150+
b.beginCollectToObjectArray();
11461151
for (int i = 0; i < cellVariableLocals.length; i++) {
11471152
b.beginCreateCell();
11481153
if (scope.getUseOfName(cellVariables[i]).contains(DefUse.DefParam)) {
@@ -1159,7 +1164,7 @@ public void setUpFrame(ArgumentsTy args, Builder b) {
11591164
}
11601165
b.endCreateCell();
11611166
}
1162-
b.endMakeVariadic();
1167+
b.endCollectToObjectArray();
11631168
b.endStoreRange();
11641169

11651170
for (BytecodeLocal local : toClear) {
@@ -1722,11 +1727,11 @@ private void createConstant(ConstantValue value) {
17221727
break;
17231728
case TUPLE:
17241729
b.beginMakeTuple();
1725-
b.beginMakeVariadic();
1730+
b.beginCollectToObjectArray();
17261731
for (ConstantValue cv : value.getTupleElements()) {
17271732
createConstant(cv);
17281733
}
1729-
b.endMakeVariadic();
1734+
b.endCollectToObjectArray();
17301735
b.endMakeTuple();
17311736
break;
17321737
case FROZENSET:
@@ -1767,7 +1772,8 @@ public Void visit(ExprTy.Dict node) {
17671772
boolean newStatement = beginSourceSection(node, b);
17681773

17691774
if (len(node.keys) == 0) {
1770-
b.emitMakeEmptyDict();
1775+
b.beginMakeDict(0);
1776+
b.endMakeDict();
17711777
} else {
17721778
b.beginMakeDict(node.keys.length);
17731779
for (int i = 0; i < node.keys.length; i++) {
@@ -2027,11 +2033,11 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
20272033
*
20282034
* @formatter:off
20292035
* Unstar(
2030-
* MakeVariadic(a, b),
2036+
* CollectToObjectArray(a, b),
20312037
* UnpackStarred(c),
2032-
* MakeVariadic(d, e),
2038+
* CollectToObjectArray(d, e),
20332039
* UnpackStarred(f),
2034-
* MakeVariadic(g)
2040+
* CollectToObjectArray(g)
20352041
* )
20362042
* @formatter:on
20372043
*/
@@ -2040,15 +2046,15 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
20402046
int numOperands = 0;
20412047

20422048
if (initialElementsProducer != null) {
2043-
b.beginMakeVariadic();
2049+
b.beginCollectToObjectArray();
20442050
initialElementsProducer.run();
20452051
inVariadic = true;
20462052
}
20472053

20482054
for (int i = 0; i < args.length; i++) {
20492055
if (args[i] instanceof ExprTy.Starred) {
20502056
if (inVariadic) {
2051-
b.endMakeVariadic();
2057+
b.endCollectToObjectArray();
20522058
inVariadic = false;
20532059
numOperands++;
20542060
}
@@ -2059,7 +2065,7 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
20592065
numOperands++;
20602066
} else {
20612067
if (!inVariadic) {
2062-
b.beginMakeVariadic();
2068+
b.beginCollectToObjectArray();
20632069
inVariadic = true;
20642070
}
20652071

@@ -2068,33 +2074,31 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
20682074
}
20692075

20702076
if (inVariadic) {
2071-
b.endMakeVariadic();
2077+
b.endCollectToObjectArray();
20722078
numOperands++;
20732079
}
20742080

20752081
b.endUnstar(numOperands);
20762082
} else {
2077-
b.beginMakeVariadic();
2083+
b.beginCollectToObjectArray();
20782084
if (initialElementsProducer != null) {
20792085
initialElementsProducer.run();
20802086
}
20812087
visitSequence(args);
2082-
b.endMakeVariadic();
2088+
b.endCollectToObjectArray();
20832089
}
20842090
}
20852091

20862092
@Override
20872093
public Void visit(ExprTy.Set node) {
20882094
boolean newStatement = beginSourceSection(node, b);
2089-
2095+
b.beginMakeSet();
20902096
if (len(node.elements) == 0) {
2091-
b.emitMakeEmptySet();
2097+
b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
20922098
} else {
2093-
b.beginMakeSet();
20942099
emitUnstar(node.elements);
2095-
b.endMakeSet();
20962100
}
2097-
2101+
b.endMakeSet();
20982102
endSourceSection(b, newStatement);
20992103
return null;
21002104
}
@@ -2943,7 +2947,8 @@ private void emitKeywordGroup(KeywordGroup group, boolean copy, BytecodeLocal fu
29432947

29442948
if (copy) {
29452949
b.beginKwargsMerge(function);
2946-
b.emitMakeEmptyDict();
2950+
b.beginMakeDict(0);
2951+
b.endMakeDict();
29472952
splatKeywords.expr.accept(this);
29482953
b.endKwargsMerge();
29492954
} else {
@@ -3229,11 +3234,11 @@ private void emitMakeFunction(SSTNode node, String name, ArgumentsTy args, List<
32293234
if (args == null || len(args.defaults) == 0) {
32303235
b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
32313236
} else {
3232-
b.beginMakeVariadic();
3237+
b.beginCollectToObjectArray();
32333238
for (int i = 0; i < args.defaults.length; i++) {
32343239
args.defaults[i].accept(this);
32353240
}
3236-
b.endMakeVariadic();
3241+
b.endCollectToObjectArray();
32373242
}
32383243

32393244
boolean hasKeywords = false;
@@ -3822,9 +3827,9 @@ private void doVisitPattern(PatternTy.MatchSequence node, PatternContext pc) {
38223827

38233828
b.beginPrimitiveBoolAnd();
38243829

3825-
b.beginIsSequence();
3830+
b.beginCheckTypeFlags(TypeFlags.SEQUENCE);
38263831
b.emitLoadLocal(pc.subject);
3827-
b.endIsSequence();
3832+
b.endCheckTypeFlags();
38283833

38293834
if (star < 0) {
38303835
// No star: len(subject) == size

0 commit comments

Comments
 (0)