[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

lattner added a comment. > @lattner, thanks for the help. In this case, the real question is whether > there's any use case for !srcloc that involves writing it out into a bitcode > or IR file and then having a separate instance of clang load it back in again. > I think that no such case can

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. (But I've pushed this patch anyway, because I'm reasonably confident of the answer; if it turns out that there's some case along those lines that I should have taken care of, I'll fix or revert.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. simon_tatham marked an inline comment as done. Closed by commit rGbd41136746a0: [clang] Use i64 for the !srcloc metadata on asm IR nodes. (authored by simon_tatham). C

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. @lattner, thanks for the help. In this case, the real question is whether there's any use case for `!srcloc` that involves writing it out into a bitcode or IR file and then having a separate instance of clang load it back in again. I think that no such case can pos

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

lattner added a comment. > cat f.c void x() { asm("bogus"); } > clang f.c f.c:3:7: error: invalid instruction mnemonic 'bogus' asm("bogus"); ^ :1:2: note: instantiated into assembly here bogus ^ 1 error generated. Repository: rG LLVM G

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

lattner added a comment. The usecase for this is to get good diagnostics out of the integrated assembler. Try doing something like: `asm("bogus");`, GCC would produce an error pointing to the generated .s file. Clang should point to the C file (including macros expanded etc). Repository:

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

dexonsmith added a comment. In D105491#2893632 , @simon_tatham wrote: > In D105491#2891860 , @dexonsmith > wrote: > >> although it'd be good to get someone more involved in lib/CodeGen to take a >> quick look /

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a subscriber: lattner. simon_tatham added a comment. In D105491#2891860 , @dexonsmith wrote: > although it'd be good to get someone more involved in lib/CodeGen to take a > quick look / sign off (ideally someone that knows the use cas

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM (one comment in the test), although it'd be good to get someone more involved in lib/CodeGen to take a quick look / sign off (ideally someone that knows the use case for `!srcloc`

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham updated this revision to Diff 360098. simon_tatham edited the summary of this revision. simon_tatham added a comment. Added an i64 `!srcloc` to the only existing test of them I could find. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. I looked into this yesterday, and realised that I don't actually know what the use case //is// for emitting `!srcloc` metadata in an IR file. It's more or less ignored by llc, as far as I can see: if there's a late-breaking error in the inline asm string, you just

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

dexonsmith added a comment. In D105491#2882549 , @tmatheson wrote: > @dexonsmith I don't think they need upgraded. Most of the places I can see > referencing `!srcloc` are copying it around and will preserve the i32 type. > Cases which actually read the

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

tmatheson added a comment. @dexonsmith I don't think they need upgraded. Most of the places I can see referencing `!srcloc` are copying it around and will preserve the i32 type. Cases which actually read the value are already reading it as 64 bit via `getZExtValue` and have been updated here, e

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

dexonsmith added a comment. Does this need an upgrade for old bitcode that has serialized 32-bit types? If not, can you explain why not? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105491/new/ https://reviews.llvm.org/D105491 __

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

tmatheson added a comment. Looks sensible to me, I don't think slightly expanding the size of the metadata and the diagnostic will be an issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105491/new/ https://reviews.llvm.org/D105491 ___

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added subscribers: dexonsmith, hiraditya. simon_tatham requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This is part of a