Page MenuHomePhabricator

SouraVX (Sourabh Singh Tomar)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 27 2019, 11:05 PM (59 w, 5 d)

Recent Activity

Fri, Oct 16

SouraVX abandoned D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

Thanks @kiranchandramohan for helping out in this :). I'm abandoning this in favor of your approach. (Updating the PR also)
Thanks @tskeith for suggestion, I'll take a note of this.

Fri, Oct 16, 9:08 AM · Restricted Project, Restricted Project, Restricted Project
SouraVX updated subscribers of D89385: [Flang][OpenMP 4.5] Add semantic check for OpenMP copyin clause.
Fri, Oct 16, 8:55 AM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

Thanks @kiranchandramohan for suggestion. I already tried this thing(didn't worked out).
Unless I'm missing something: There's a type mismatch while interfacing this function. Here's the snippet of usability

case Fortran::parser::OmpProcBindClause::Type::Master:
          parallelOp.proc_bind_valAttr(firOpBuilder.getStringAttr(
           omp::stringifyClauseProcBindKind(procBindClause->v)));
          break;

Error:

error: cannot convert ‘const Fortran::parser::OmpProcBindClause::Type’ to ‘mlir::omp::ClauseProcBindKind’ for argument ‘1’ to ‘llvm::StringRef mlir::omp::stringifyClauseProcBindKind(mlir::omp::ClauseProcBindKind)’
              omp::stringifyClauseProcBindKind(procBindClause->v)));

An alternative solution would be: (but it's hard coding) ? (do you vote for this?)

case Fortran::parser::OmpProcBindClause::Type::Master:
          parallelOp.proc_bind_valAttr(firOpBuilder.getStringAttr(
           StringRef("master")));
          break;
Fri, Oct 16, 7:57 AM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

Ping! @ all reviewers.

Fri, Oct 16, 7:24 AM · Restricted Project, Restricted Project, Restricted Project

Thu, Oct 15

SouraVX added inline comments to D87684: [mlir]Add Allocate Clause to OMP Parallel Operation Definition.
Thu, Oct 15, 8:22 AM · Restricted Project

Wed, Oct 14

SouraVX added a comment to D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

I guess @clementval or @rriddle can approve the new extra functions added based on its need in Flang parse-tree lowering or on similar usage for StrEnumAttributes in rest of MLIR.

Snippet of usability from an upcoming/in-progress patch.

 switch (procBindClause->v) {
+                     case Fortran::parser::OmpProcBindClause::Type::Master:
+                     parallelOp.proc_bind_valAttr(firOpBuilder.getStringAttr(parallelOp.getProcBindValueMasterAttrName()));
+                             break;
+                     case Fortran::parser::OmpProcBindClause::Type::Close:
+                     parallelOp.proc_bind_valAttr(firOpBuilder.getStringAttr(parallelOp.getProcBindValueCloseAttrName()));
+                             break;
+                     case Fortran::parser::OmpProcBindClause::Type::Spread:
+                     parallelOp.proc_bind_valAttr(firOpBuilder.getStringAttr(parallelOp.getProcBindValueSpreadAttrName()));
+                             break;

Aside from this, Should we promote/remodel these attributes as UnitAttr? What do you guys think?

I feel the current modelling as an Optional StrEnumAttribute is fine here. proc_bind and default takes a set of values which are best captured by this modelling. If we have these as unit attributes then all the values can occur at the same time unless disabled using the verifier.

Wed, Oct 14, 8:05 PM · Restricted Project, Restricted Project, Restricted Project

Tue, Oct 13

SouraVX retitled D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp` from [MLIR, OpenMP] Remodel `proc_bind_kind` clause as a unit attribute to [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.
Tue, Oct 13, 10:21 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX updated the diff for D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

Ah, potential oversight of details(Sorry for that!). Anyway I've rolled back to initial version and introduced some extra declaration that can be used by Flang(once this patch is in) while lowering.
Aside from this, Should we promote/remodel these attributes as UnitAttr? What do you guys think?

Tue, Oct 13, 10:20 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

What kind of issues? This seems like a regression in terms of usability. Can you describe in detail the problems that you were having?

Tue, Oct 13, 1:53 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.

If we agreed upon this, the next would be to remodel default clause as UnitAttr also.

Tue, Oct 13, 1:39 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX requested review of D89340: [MLIR, OpenMP] Introduce extra helper function for clauses of `ParallelOp`.
Tue, Oct 13, 1:36 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O.

Thanks for the patch! Overall it's in pretty nice state and conformant to design highlighted.
Could you please clang-format this patch, There are lot of lint warning that causes lot of noise while reviewing.
Couple of NIT comments inline. I've tried marking all but, Could you please finish comments with period.

Tue, Oct 13, 3:15 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Oct 12

SouraVX added inline comments to D89218: [DebugInfo] Support for DW_TAG_generic_subrange.
Mon, Oct 12, 7:04 AM · debug-info, Restricted Project
SouraVX added a reviewer for D89218: [DebugInfo] Support for DW_TAG_generic_subrange: schweitz.
Mon, Oct 12, 7:04 AM · debug-info, Restricted Project

Fri, Oct 9

SouraVX added inline comments to D88965: [Flang][OpenMP] Rework parser changes for OpenMP atomic construct..
Fri, Oct 9, 9:24 AM · Restricted Project, Restricted Project
SouraVX added inline comments to D88965: [Flang][OpenMP] Rework parser changes for OpenMP atomic construct..
Fri, Oct 9, 5:29 AM · Restricted Project, Restricted Project

Wed, Oct 7

SouraVX added a project to D88628: [flang][openacc] Fix semantic check for wait, cache and atomic directives: Restricted Project.
Wed, Oct 7, 8:13 AM · Restricted Project, Restricted Project
SouraVX accepted D88720: [OpenMP][MLIR] Fix for nested parallel regions.

Thanks for the patch. You may want to wait for @jdoerfert for final sign off!

Wed, Oct 7, 8:02 AM · Restricted Project
SouraVX added inline comments to D88720: [OpenMP][MLIR] Fix for nested parallel regions.
Wed, Oct 7, 12:16 AM · Restricted Project

Tue, Oct 6

SouraVX accepted D88909: [NFC][flang] Introduce new helper TODO macro for further development.

Thanks for the patch!
NIT:
A more reasonable commit message could be:
[NFCI][flang] Introduce new helper TODO for further development
Or something similar(aligned to intent) :)

Tue, Oct 6, 10:12 AM · Restricted Project, Restricted Project

Mon, Oct 5

SouraVX updated subscribers of D88706: [OpenMP][MLIR] WIP : Fix for nested parallel region.
Mon, Oct 5, 12:40 AM · Restricted Project, Restricted Project

Sun, Oct 4

SouraVX added inline comments to D88706: [OpenMP][MLIR] WIP : Fix for nested parallel region.
Sun, Oct 4, 11:02 PM · Restricted Project, Restricted Project

Fri, Oct 2

SouraVX added inline comments to D88740: [mlir][openmp][NFC]Remove unnecessary brackets and rephrase ParallelOp description in mlir definition.
Fri, Oct 2, 12:51 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a comment to D88720: [OpenMP][MLIR] Fix for nested parallel regions.

@SouraVX In this patch I have removed the OpenMPIRBuilder alloca changes. This contains only the changes to in ModuleTranslation and it seems to work fine. Can you check at your end as well as with the master Operation?

Fri, Oct 2, 12:16 PM · Restricted Project

Tue, Sep 29

SouraVX accepted D87684: [mlir]Add Allocate Clause to OMP Parallel Operation Definition.

Since it's already approved, feel free to remove test case modification and commit no need for another revision :)

Tue, Sep 29, 4:44 AM · Restricted Project

Fri, Sep 25

SouraVX committed rGd2f1f530430e: [flang][OpenMP] Place the insertion point to the start of the block (authored by SouraVX).
[flang][OpenMP] Place the insertion point to the start of the block
Fri, Sep 25, 11:27 AM
SouraVX closed D88221: [flang][OpenMP] Place the insertion point to the start of the block.
Fri, Sep 25, 11:27 AM · Restricted Project, Restricted Project

Thu, Sep 24

SouraVX retitled D88221: [flang][OpenMP] Place the insertion point to the start of the block from [flang][OpenMP] Upstream OpenMP Parallel construct codegen support to [flang][OpenMP] Place the insertion point to the start of the block.
Thu, Sep 24, 10:16 AM · Restricted Project, Restricted Project
SouraVX added a comment to D88221: [flang][OpenMP] Place the insertion point to the start of the block.

Are you alluding, that this review should be renamed as some "Upstream XXXX code from PR" and nothing else ?

Thu, Sep 24, 7:43 AM · Restricted Project, Restricted Project
SouraVX added a comment to D88221: [flang][OpenMP] Place the insertion point to the start of the block.

This is not "codegen support" is it? That sounds like "we now support codegen of X". Maybe: "Fix XXX codegen insertion point placement"?

Thu, Sep 24, 7:41 AM · Restricted Project, Restricted Project
SouraVX added a comment to D88221: [flang][OpenMP] Place the insertion point to the start of the block.

Please use a proper commit message and subject. Linking some other discussion is fine but it should be self contained.

Sorry, I should've done better. Now modified to convey the intent.

Thu, Sep 24, 7:04 AM · Restricted Project, Restricted Project
SouraVX retitled D88221: [flang][OpenMP] Place the insertion point to the start of the block from [flang][OpenMP] Parallel region codegen support to [flang][OpenMP] Upstream OpenMP Parallel construct codegen support.
Thu, Sep 24, 7:03 AM · Restricted Project, Restricted Project
SouraVX added a project to D88221: [flang][OpenMP] Place the insertion point to the start of the block: Restricted Project.
Thu, Sep 24, 5:26 AM · Restricted Project, Restricted Project
SouraVX requested review of D88221: [flang][OpenMP] Place the insertion point to the start of the block.
Thu, Sep 24, 5:26 AM · Restricted Project, Restricted Project

Wed, Sep 23

SouraVX committed rGdfa9065ad778: [NFCI][flang] Renamed a variable name to a more descriptive name (authored by SouraVX).
[NFCI][flang] Renamed a variable name to a more descriptive name
Wed, Sep 23, 5:54 AM
SouraVX added a comment to D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.

Removed unit-test in be1197c403b22291e35cbc5e96788860ceabd40c

Wed, Sep 23, 5:47 AM · Restricted Project, Restricted Project
SouraVX committed rGbe1197c403b2: [flang] Removed OpenMP lowering unittests (authored by SouraVX).
[flang] Removed OpenMP lowering unittests
Wed, Sep 23, 5:47 AM
SouraVX committed rG34b08487f04a: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct (authored by SouraVX).
[OpenMP][flang]Lower NUM_THREADS clause for parallel construct
Wed, Sep 23, 5:28 AM
SouraVX closed D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Wed, Sep 23, 5:27 AM · Restricted Project, Restricted Project
SouraVX updated the summary of D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Wed, Sep 23, 5:25 AM · Restricted Project, Restricted Project
SouraVX added a comment to D87549: [OpenMP][MLIR] Add assembly format for master op.

Reverse Ping!

Wed, Sep 23, 1:00 AM · Restricted Project

Tue, Sep 22

SouraVX updated the diff for D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Tue, Sep 22, 12:57 AM · Restricted Project, Restricted Project
SouraVX added a reviewer for D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct: jeanPerier.
Tue, Sep 22, 12:28 AM · Restricted Project, Restricted Project
SouraVX added a comment to D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.

Thanks @jeanPerier for reviewing this!

Tue, Sep 22, 12:27 AM · Restricted Project, Restricted Project

Mon, Sep 21

SouraVX added a project to D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs.: Restricted Project.
Mon, Sep 21, 12:30 AM · Restricted Project, Restricted Project, Restricted Project
SouraVX added a reviewer for D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs.: schweitz.
Mon, Sep 21, 12:29 AM · Restricted Project, Restricted Project, Restricted Project
SouraVX added reviewers for D87908: [flang] [OpenMP 4.5] Adding lit test cases for OpenMP Constructs.: kiranktp, SouraVX.
Mon, Sep 21, 12:29 AM · Restricted Project, Restricted Project, Restricted Project

Sun, Sep 20

SouraVX committed rG000eb1f314c1: [docs][flang] Fix typos (authored by SuJunda).
[docs][flang] Fix typos
Sun, Sep 20, 10:36 PM
SouraVX closed D87885: [docs][flang] Fix typos.
Sun, Sep 20, 10:36 PM · Restricted Project, Restricted Project
SouraVX added a comment to D87885: [docs][flang] Fix typos.
Sun, Sep 20, 10:26 PM · Restricted Project, Restricted Project
SouraVX retitled D87885: [docs][flang] Fix typos from [docs] Fix typos to [docs][flang] Fix typos.
Sun, Sep 20, 10:24 PM · Restricted Project, Restricted Project
SouraVX accepted D87885: [docs][flang] Fix typos.

LGTM, thanks!

Sun, Sep 20, 10:23 PM · Restricted Project, Restricted Project

Sep 18 2020

SouraVX added inline comments to D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Sep 18 2020, 2:08 PM · Restricted Project, Restricted Project

Sep 17 2020

SouraVX added inline comments to D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Sep 17 2020, 9:43 PM · Restricted Project, Restricted Project
SouraVX requested review of D87846: [OpenMP][flang]Lower NUM_THREADS clause for parallel construct.
Sep 17 2020, 10:57 AM · Restricted Project, Restricted Project

Sep 13 2020

SouraVX added inline comments to D87147: PR-47391 : Two DIFile entries are describing the same file two different ways.
Sep 13 2020, 9:51 PM · debug-info
SouraVX added a reviewer for D87500: [DebugInfo] DISubrange support for fortran assumed size array: schweitz.
Sep 13 2020, 9:17 AM · Restricted Project, debug-info

Sep 11 2020

SouraVX added inline comments to D87549: [OpenMP][MLIR] Add assembly format for master op.
Sep 11 2020, 10:22 PM · Restricted Project
SouraVX accepted D87549: [OpenMP][MLIR] Add assembly format for master op.

Thanks @kiranchandramohan for pushing this separately.
LGTM :)

Sep 11 2020, 10:20 PM · Restricted Project

Sep 10 2020

SouraVX committed rGe45b0708ae81: [DebugInfo] Fixing CodeView assert related to lowerBound field of DISubrange. (authored by alok).
[DebugInfo] Fixing CodeView assert related to lowerBound field of DISubrange.
Sep 10 2020, 10:44 PM
SouraVX closed D87406: [DebugInfo] Fixing CodeView assert related to lowerBound field of DISubrange..
Sep 10 2020, 10:44 PM · Restricted Project, debug-info
SouraVX added a comment to D86875: [Flang][NFC] Remove license comments from files in docs/ folder..

Revereted!
Thanks!

Sep 10 2020, 10:37 AM · Restricted Project
SouraVX added a reverting change for D86875: [Flang][NFC] Remove license comments from files in docs/ folder.: rG932aae77e92b: Revert D86875 "[Flang][NFC] Remove license comments from files in docs/ folder.".
Sep 10 2020, 10:37 AM · Restricted Project
SouraVX added a reverting change for rGf787c9a90c69: [Flang][NFC] Remove license comments from files in docs/ folder.: rG932aae77e92b: Revert D86875 "[Flang][NFC] Remove license comments from files in docs/ folder.".
Sep 10 2020, 10:37 AM
SouraVX committed rG932aae77e92b: Revert D86875 "[Flang][NFC] Remove license comments from files in docs/ folder." (authored by SouraVX).
Revert D86875 "[Flang][NFC] Remove license comments from files in docs/ folder."
Sep 10 2020, 10:37 AM
SouraVX added a comment to D87389: [flang][openacc] Lower clauses on loop construct to OpenACC dialect.

Yes those are duplicate in a sense, but they are still tests(since nothing else is there) and can self validate well-formedness of the operation(which is one of the major pre-requisite for proceeding further with Lowering).
+ They will ensure matches with community criteria. up-streaming in testable chunks.

My principal concern is that they do not test the code in genACC or genOMP so they won't catch any problem or regression there.

I think that is justified and already tested with full coverage in regression(end-to-end) test with bbc.

I'm trying to put in place a unit test that make use of the genACC function directly. If that works I'll add the test before landing the patch.

+1
The reason why I'm saying this that we(you) also don't want both branches going out-of-sync and regressing further development :)

Sep 10 2020, 7:22 AM · Restricted Project, Restricted Project

Sep 9 2020

SouraVX removed a project from D87389: [flang][openacc] Lower clauses on loop construct to OpenACC dialect: Restricted Project.
Sep 9 2020, 11:28 PM · Restricted Project, Restricted Project
SouraVX accepted D87389: [flang][openacc] Lower clauses on loop construct to OpenACC dialect.

Since bbc based test can't be added here, would you mind adding a unit-test here ?

Well I have to see how to add a meaningful unit test here. Lowering unit tests that are currently upstreamed seem to just copy-paste the lowering code which does not really test the lowering code itself.

Sep 9 2020, 11:26 PM · Restricted Project, Restricted Project
SouraVX added a comment to D87389: [flang][openacc] Lower clauses on loop construct to OpenACC dialect.

Since bbc based test can't be added here, would you mind adding a unit-test here ?

Sep 9 2020, 10:50 AM · Restricted Project, Restricted Project
SouraVX edited projects for D87389: [flang][openacc] Lower clauses on loop construct to OpenACC dialect, added: Restricted Project; removed Restricted Project.
Sep 9 2020, 10:48 AM · Restricted Project, Restricted Project

Sep 8 2020

SouraVX added a reviewer for D87247: [MLIR,OpenMP] Added support for lowering MasterOp to LLVMIR: fghanim.
Sep 8 2020, 8:33 AM · Restricted Project
SouraVX added a comment to D87247: [MLIR,OpenMP] Added support for lowering MasterOp to LLVMIR.

Thanks @kiranktp ,@kiranchandramohan for reviewing this!
When adding a test case for a MasterOp inside the region of ParallelOp(typical use case.)
Test snippet:

llvm.func @test_omp_master() -> () {
  omp.parallel {
  "omp.master" ()({
    omp.terminator
  }):()->()
    omp.terminator
}
  llvm.return
}

This produces following crash/assertion failure in OMPIRBuilder:

mlir-translate: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:717: llvm::IRBuilderBase::InsertPoint llvm::OpenMPIRBuilder::CreateParallel(const llvm::OpenMPIRBuilder::LocationDescription&, llvm::OpenMPIRBuilder::InsertPointTy, llvm::OpenMPIRBuilder::BodyGenCallbackTy, llvm::OpenMPIRBuilder::PrivatizeCallbackTy, llvm::OpenMPIRBuilder::FinalizeCallbackTy, llvm::Value*, llvm::Value*, llvm::omp::ProcBindKind, bool): Assertion `Outputs.empty() && "OpenMP outlining should not produce live-out values!"' failed.

digging in further, I realized this Outputs is getting populated with(below instruction) and finally asserting at Outputs.empty().

%omp_global_thread_num2 = call i32 @__kmpc_global_thread_num(%struct.ident_t* @3), !dbg !9

I'll dig further, BTW if you guys have any inputs WRT this. Please share!

Sep 8 2020, 8:33 AM · Restricted Project

Sep 7 2020

SouraVX added a reviewer for D87247: [MLIR,OpenMP] Added support for lowering MasterOp to LLVMIR: kiranktp.
Sep 7 2020, 10:36 AM · Restricted Project
SouraVX requested review of D87247: [MLIR,OpenMP] Added support for lowering MasterOp to LLVMIR.
Sep 7 2020, 10:35 AM · Restricted Project

Sep 6 2020

SouraVX added a reviewer for D87147: PR-47391 : Two DIFile entries are describing the same file two different ways: aprantl.
Sep 6 2020, 10:28 PM · debug-info

Sep 1 2020

SouraVX committed rG03812041d8d9: [NFCI] Removed an un-used declaration got accidentally introduced in… (authored by SouraVX).
[NFCI] Removed an un-used declaration got accidentally introduced in…
Sep 1 2020, 3:30 AM

Aug 31 2020

SouraVX committed rGdb464a2753e2: [NFCI] Silent a build warning due to an extra semi-colon (authored by SouraVX).
[NFCI] Silent a build warning due to an extra semi-colon
Aug 31 2020, 5:20 AM

Aug 23 2020

SouraVX added a reviewer for D86089: [flang][driver]Add experimental flang driver and frontend with help screen: rsmith.
Aug 23 2020, 10:54 PM · Restricted Project, Restricted Project, Restricted Project
SouraVX added inline comments to D86089: [flang][driver]Add experimental flang driver and frontend with help screen.
Aug 23 2020, 10:53 PM · Restricted Project, Restricted Project, Restricted Project

Aug 21 2020

SouraVX committed rG12edd4b36475: Fix arm bot failure after f91d18eaa946b2 (authored by SouraVX).
Fix arm bot failure after f91d18eaa946b2
Aug 21 2020, 10:44 PM
SouraVX updated the summary of D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 21 2020, 9:54 PM · debug-info, Restricted Project
SouraVX updated the summary of D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 21 2020, 9:52 PM · debug-info, Restricted Project
SouraVX added a comment to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

I don't know what's the criteria for arc land for removing strings from commit message. I added 2 Co-author: Eric Schweitz and Bhuvanendra kumar(from AMD). But apparently it dropped it when landing the actual patch. :(
Are you guys Okay with this ?
Anyway, I'll modify the summary here as well(for attributing co-author's).

Aug 21 2020, 9:50 PM · debug-info, Restricted Project
SouraVX committed rGf91d18eaa946: [DebugInfo][flang]Added support for representing Fortran assumed length strings (authored by SouraVX).
[DebugInfo][flang]Added support for representing Fortran assumed length strings
Aug 21 2020, 9:44 PM
SouraVX closed D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 21 2020, 9:44 PM · debug-info, Restricted Project
SouraVX updated the diff for D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

Added comments for reserved bit codes.

Aug 21 2020, 1:24 PM · debug-info, Restricted Project
SouraVX added a comment to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

I'm fine with whatever numbering makes your development the easiest. My only ask is that if we still have a gap after this, please document what nodes are expected to go there, so there 1) won't be any future conflicts and 2) we don't pointlessly reserve space.

I'll have to re-confirm with our team. Making sure we don't break anything is priority for us.
@schweitz do you have any reservations WRT this ?

Aug 21 2020, 10:48 AM · debug-info, Restricted Project
SouraVX added a comment to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

I'm fine with whatever numbering makes your development the easiest. My only ask is that if we still have a gap after this, please document what nodes are expected to go there, so there 1) won't be any future conflicts and 2) we don't pointlessly reserve space.

Aug 21 2020, 10:09 AM · debug-info, Restricted Project
SouraVX added a comment to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

You might want to add "[flang]" to the subject line as this change supports Fortran. Thanx.

Aug 21 2020, 9:12 AM · debug-info, Restricted Project
SouraVX retitled D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings from [DebugInfo]Added support for representing Fortran assumed length strings to [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 21 2020, 9:11 AM · debug-info, Restricted Project
SouraVX updated the diff for D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

Updated BitCodes for DIString to 41, this will narrow down the gap. Moving forward when introducing new metadata we can use 42-43 bits to compensate the gap.

Aug 21 2020, 7:39 AM · debug-info, Restricted Project

Aug 20 2020

SouraVX added inline comments to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 20 2020, 10:43 PM · debug-info, Restricted Project
SouraVX added a comment to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

Thanks @aprantl for reviewing this. Let's wait for @schweitz also to have a look. Planning to land this tomorrow or so.

Aug 20 2020, 1:31 PM · debug-info, Restricted Project
SouraVX added inline comments to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 20 2020, 1:25 PM · debug-info, Restricted Project
SouraVX updated the diff for D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.

Addressed @aprantl review comments. Thanks for reviewing this!
Changes:

  • Reduced to minimal variants.
  • And other Nit comments.
Aug 20 2020, 1:21 PM · debug-info, Restricted Project
SouraVX added inline comments to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 20 2020, 11:09 AM · debug-info, Restricted Project
SouraVX added inline comments to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 20 2020, 9:56 AM · debug-info, Restricted Project
SouraVX added a project to D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings: debug-info.
Aug 20 2020, 9:53 AM · debug-info, Restricted Project
SouraVX requested review of D86305: [DebugInfo][flang]Added support for representing Fortran assumed length strings.
Aug 20 2020, 9:53 AM · debug-info, Restricted Project
SouraVX added a comment to D86272: [DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian.

Thanks @bjope for catching this bug! I do not have big-endian machine so it must've got un-noticed(test case for 4 bytes float was written and tested/verified on x86 machine).
But I got your point, I'll see if I can add a hand written test case to address this situation.

Aug 20 2020, 3:29 AM · Restricted Project