rogfer01 added inline comments.

================
Comment at: clang/lib/Sema/SemaLookup.cpp:923
+
+  const RVVIntrinsicInfo *Intrinsic = std::find_if(
+      std::begin(RVVIntrinsicInfos), std::end(RVVIntrinsicInfos),
----------------
Not for this patch: I think this table may be a bit large so all lookups 
(including those that will fail) will be slower after a `#pragma riscv 
intrinsic vector` is found.

Filtering them as fast as possible (looking at the spec shows that currently 
all RVV intrinsics start with `v`) or using some hash table (if too difficult 
to build at compile time we could build it the first time we get here?) might 
be something we want to do.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:1011
 
+      if (PP.getPredefines() == "#define __riscv_pragma_vector_intrinsics") {
+        const TargetInfo &TI = Context.getTargetInfo();
----------------
This seems a bit fragile if there are more predefines than just this one. I 
understand the intent is to avoid looking up the RVV builtin every time, only 
do that if we have found the pragma, right?

Several pragma handlers receive a reference to `Sema` (in an object called 
`Action` or `Actions`) and then they notify `Sema` (via a member function that 
would have to be added to it) about having parsed the pragma. That could be 
used to set some flag to true in `Sema` itself and also emit diagnostics if we 
want (e.g. what if the pragma is used twice? can it be used anywhere?).

Do you think this would be workable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111617/new/

https://reviews.llvm.org/D111617

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to