[cpl] [Tema 3] Feedback
Andrei Tuicu
andrei.tuicu at gmail.com
Fri Jan 8 10:17:17 EET 2016
Salut!
Am sa incerc sa sintetizez in acest mail problemele intalnite in efectuarea
temei 3.
In primul rand am sa incep prin a-mi cere scuze daca nu voi mentine o
perspectiva strict obiectiva in acest mail. De asemenea voi folosi termenul
“echipa”, deoarece nu stiu pentru fiecare parte a temei cine a fost
persoana responsabila.
-
Publicarea temei:
Desi enuntul temei a fost publicat aproximativ la data anuntata in
prealabil, arhiva de pornire a fost publicata in saptamana dinaintea
vacantei, deadline-ul fiind fixat pentru prima zi dupa vacanta (4
ianuarie). Acest lucru insemna ca in mod oficial, fara vacanta, studentii
aveau la dispozitie cam o saptamana pentru rezolvarea temei.
-
Enuntul (recunosc ca l-am citit inainte sa ma apuc de tema, nu mi s-a
parut prea folositor si l-am recitit din nou numai in momentul in care am
scris acest email):
Desi sunt destul de multe explicatii in acesta, exista totusi informatii
care lipsesc si care ar fi putut fi de mare ajutor. De exemplu, modul
corect de generarea a structurilor T<Class> si R<Class>. Faptul ca in
generarea structurii T<Class> intai se copiaza atributele din clasa
parinte, abea apoi se adauga atributele clasei derivate. Similar, cred ca
ar fi fost de ajutor o descriere a modului in care trebuie generat
vtable-ul. (Se adaugau pe rand metodele clasei parinte, ce erau inlocuite
in cazul in care o metoda era suprascrisa, pastrandu-se astfel ordinea si
prin aceasta indexul din vtable la care se afla o metoda; la final se
adaugau metodele definite de clasa derivata, ce nu suprascriau metode din
clasa parinte).
Exista de asemenea si cateva greseli in enunt. In primul rand offset-urile
din structurile TClass si RClass, offset-uri ce nu sunt oricum de prea mare
ajutor deoarece lucrand cu GetElementPtrInst, acesta nu are nevoie decat de
index-ul in cadrul structurii. De asemenea nu era corecta sectiunea de
inlocuire a caracterelor speciale. Acestea nu trebuiau inlocuite cu
valoarea lor in hexazecimal, ci trebuiau identificate si scoase escaparile,
iar lista nu era completa. ( ex. “\\n” trece in “\n”, “\\\\” trece in “\\”,
“\\” trece in “”, etc).
-
Arhiva de pornire
-
Makefile-ul:
Nu cunosc niciun caz in care cineva sa fi putut compila tema folosind
regula de build, nici macar pe masina virtuala. Am auzit ca exista o
posibilitate sa mearga pe MacOS, dar nu stiu niciun caz concret.
Regula de clean:
rm ASTCodeGen.cpp main.cpp lcpl-codegen
-
Scheletul de cod:
In primul rand, tema depindea de tema 2, dar in arhiva se afla numai
scheletul pentru aceasta si nu era specificat nicaieri acest lucru. Mai
mult, acest lucru nu se observa dintr-un stadiu incipient, ci dupa cateva
zile de implementare cand se ajungea la evaluarea simbolurilor,
assignment-urilor, etc. Mi-am dat seama ca intr-adevar trebuie sa ma
folosesc de tema 2 cand am vazut ca este trimis ca parametru
constructorului clasei ASTCodeGen un obiect de tip SemanticAnalysis, iar
acest lucru s-a intamplat dupa ce am stat si am incercat sa-mi dau seama
cum as putea sa fac implementarea fara informatiile furnizate de analiza
semantica si fara sa folosesc SymbolTable.
Posibila solutie: furnizarea unei biblioteci cu implementarea completa si
corecta a temei2. O alta solutie, in opinia mea nu la fel de buna, ar fi
anuntarea in momentul publicarii temei 2 de aceasta dependenta intre cele
doua teme.
In al doilea rand, s-a remarcat lipsa TODO-urilor. Concret, existau cateva
comentarii marcate cu TODO. Pe alocuri mai existau si alte hint-uri in
comentarii despre ce trebuie facut, dar acestea erau incomplete si uneori
chiar eronate. (Exemple: comentariu in visit(BasicBlock*) care sugera
crearea unei instante de BasicBlock, ce nu trebuia de fapt scris acolo ci
in alte metode, deoarece nu se puteau face branch-urile fara referinte la
blocurile respective. Comentariile din visit(IfStatement) care daca erau
urmate duceau la o inlantuire gresita a basic block-urilor in IR-ul final,
in cazul in care aveam if in if, while, etc. - Lista nu este exhaustiva).
Hardcode-arile. Desi era specificat clar ca scheletul contine numai
instructiunile pentru rularea testului hello.lcpl, consider, mai ales
pentru constantele numerice hardcodate ca parametri ai diferitelor metode
din API-ul llvm, ca era nevoie fie de o semnalare, fie in cazul ideal,
inlocuite cu instructiuni/apeluri din care ar fi rezultat acele constante.
Coding style. :) Desi nu voi insista pe acest punct, pentru ca datorita
timpului indelungat pe care l-am petrecut rezolvand aceasta tema, recunosc
ca nu am dat nici eu o atentie deosebita acestuia, trebuie totusi mentionat
ca mi-a fost mai greu decat era necesar sa inteleg ce se intampla in
schelet, in primul rand datorita modului de denumire a variabilelor,
ordonarii instructiunilor si lipsei comentariilor.
Lista ar mai putea continua in acest punct cu unele probleme sau omisiuni
care pot fi puse totusi pe seama faptului ca scheletul era menit sa
genereze strict codul pentru hello.lcpl. (Spre exemplu lipsa declararii
prototipurilor celorlalte functii definite de runtime, dar care nu erau
folosite de acest test).
Sugestii pentru a reduce volumul de munca:
Completarea scheletului cu structuri de date ce mentin informatii despre
ordinea si indecsii in structura T<Class> a atributelor si a metodelor in
vtable.
Completarea clasei SymbolTable cu metode si structuri de date pentru
maparea simbol-urilor catre adresele acestora.
Completarea scheletului cu codul ce creaza Scope-urile in SymbolTable,
deoarece acesta a fost evaluat in cadrul temei 2.
Avand metoda visit(StringConstant) implementata in schelet se putea face si
inlocuirea caracterelor escapate, deoarece acest detaliu nu aducea niciun
plus de intelegere, dupa parerea mea, a modului in care se face generarea
de cod pentru un limbaj, dar consuma timp pentru implementare.
-
Alte resurse
-
Runtime-ul: o resursa cu adevarat utila, ce m-a ajutat sa-mi dau seama
cum trebuie sa arate structurile pentru T<Class> si R<Class>. Acesta s-a
remarcat prin claritatea codului. (o mentiune minuscula: as redenumi
__lcpl_cast in __lcpl_checkCast, deoarece in urma apelului mai este nevoie
de o instructiune BitCastInst).
-
La ultimul curs, inainte de vacanta, am solicitat fisierele .ll de
referinta, care desi orientative, ar fi fost de ajutor prin faptul ca
ofereau o directie in momentul in care studentul ramanea blocat sau nu-si
dadea seama ce este gresit in implementarea sa. Domnul profesor a fost de
acord si a ramas stabilit ca vom avea acces la aceste resurse. Nu numai ca
nu au fost publicate nici pana la data trimiterii acestui email, ci mai
mult au fost puse niste fisiere incorecte, ce erau rezultate din rularea
scheletului de cod pe testele pe care acesta nu esua. Intregul schimg de
email-uri poate fi gasit pe lista de discutii avand subiectul “[Tema3]
Checker” . (*Pin1, pentru referinte ulterioare) Din modul in care s-a
desfasurat discutia acolo, noi presupunem ca nu exista o implementare
completa si corecta in cadrul echipei CPL a acestei teme (au mai fost si
alte situatii care ne-au determinat sa tragem acesta concluzie, dar aceasta
este cea mai reprezentativa).
-
Testele: Primul lucru pe care as vrea sa-l semnalez este numarul redus
de teste cu adevarat simple. Practic, modul in care au fost concepute
aceste teste facea ca prin numarul lor redus sa se verifice cam toate
subtilitatile limbajului LCPL. Totusi, acest lucru ingreuna obtinerea
punctajului. O posibila solutie ar fi crearea mai multor teste ce verifica
implementarea unor aspecte punctuale, studentul putand obtine astfel mai
usor punctaje intermediare. Au existat de asemenea si cateva greseli in
acestea (fapt ce in alt context nu ar fi contat atat de mult). Ex.
advanced/stack (si eu zic /advanced/strlit , dar nu sunt 100% sigur).
-
VMChecker-ul: a fost functional prima data in jurul datei de 22
decembrie, dar testele corecte au fost disponibile pe 4 ianuarie.
-
Manualul pentru limbajul LCPL: aici s-au remarcat cateva omisiuni:
-
Variabilele avand tipul String sunt initializate cu string-ul vid, nu
cu null.
-
Intr-o clasa pot exista referinte la clase a caror definitie se
gaseste ulterior in fisier.
-
Initializarea variabilelor/atributelor se face intai cu valoarea 0
pentru tipul de date respectiv si doar ulterior se evalueaza
instructiunile
de initializare.
-
Suportul pe lista de discutii
Raspunsurile pe lista de discutii nu numai ca au fost tardive (veneau la
cel putin 2-3 zile de la momentul publicarii intrebarii; este de inteles
faptul ca era vacanta, dar in acelasi timp si noi studentii eram tot in
vacanta), dar au fost si incomplete, avand un caracter mai degraba general
si subliniind lucruri intuitive, mai ales in locurile unde se solicita un
raspuns punctual, locuri in care efectiv nu stiam ce ar trebui facut si nu
exista nicio alta resursa care sa ma indrume spre solutia corecta. Situatia
s-a imbunatatit spre final prin interventiile Laurei Vasilescu si a lui
Razvan Crainea.
Este posibil sa fi omis anumite probleme, fie pentru ca le-am uitat fie
pentru ca le-am considerat neimportante fata de cele susmentionate. Ii rog
pe colegii mei sa aduga completari acolo unde considera ca este necesar.
Toate acestea au facut ca volumul de munca pentru tema 3 sa fie imens,
pentru mine si colegii cu care am vorbit, insumand un numar de peste 100 de
ore (10 zile in care am lucrat intre 8 si 14 ore / zi), care depaseste
numarul insumat de ore petrecut in cadrul cursurilor, orelor de laborator
si temelor precedente ( 5 ore / saptamana * 13 saptamani + in jur de 10 ore
tema 2 + in jur de 15 ore tema 1 = 90 ore). Acest numar de ore si
problemele susmentionate cred ca provin dintr-o estimare eronata a
volumului de munca, pe care eu o pun in primul rand pe seama presupunerii
facute anterior. (a se vedea *Pin1 : aceasta tema nu a fost implementata
pana la capat pentru a se vedea care este timpul necesar).
In acest conext, as dori sa solicit o scalare a punctajului acestei teme
astfel incat sa reflecte mai bine volumul de munca.
(Imi cer scuze pentru eventualele inexactitati sau greseli gramaticale, am
incercat sa trimit mailul cat mai repede.)
Multumesc,
Andrei Tuicu 341C3
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cursuri.cs.pub.ro/pipermail/cpl/attachments/20160108/90fc421e/attachment-0001.html>
More information about the cpl
mailing list